From f4b2c4801bd2327309943a879b37bdcffafb32f1 Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Tue, 29 Apr 2025 19:11:41 +0530 Subject: [PATCH 1/6] maintained actions --- go.mod | 8 +- go.sum | 6 +- .../workflow/maintainedactions/doubts.txt | 6 + .../maintainedactions/getlatestrelease.go | 40 ++ .../maintainedactions/maintainedActions.go | 150 ++++++ .../maintainedactions/maintainedActions.json | 497 ++++++++++++++++++ .../maintainedactions_test.go | 108 ++++ .../workflow/permissions/permissions.go | 25 +- remediation/workflow/secureworkflow.go | 13 +- remediation/workflow/secureworkflow_test.go | 70 ++- .../maintainedActions/input/doubleJob.yml | 31 ++ .../input/noChangesNeeded.yml | 17 + .../maintainedActions/output/doubleJob.yml | 31 ++ .../output/noChangesNeeded.yml | 17 + testfiles/secureworkflow/input/oneJob.yml | 26 + testfiles/secureworkflow/output/oneJob.yml | 36 ++ 16 files changed, 1057 insertions(+), 24 deletions(-) create mode 100644 remediation/workflow/maintainedactions/doubts.txt create mode 100644 remediation/workflow/maintainedactions/getlatestrelease.go create mode 100644 remediation/workflow/maintainedactions/maintainedActions.go create mode 100644 remediation/workflow/maintainedactions/maintainedActions.json create mode 100644 remediation/workflow/maintainedactions/maintainedactions_test.go create mode 100644 testfiles/maintainedActions/input/doubleJob.yml create mode 100644 testfiles/maintainedActions/input/noChangesNeeded.yml create mode 100644 testfiles/maintainedActions/output/doubleJob.yml create mode 100644 testfiles/maintainedActions/output/noChangesNeeded.yml create mode 100644 testfiles/secureworkflow/input/oneJob.yml create mode 100644 testfiles/secureworkflow/output/oneJob.yml diff --git a/go.mod b/go.mod index c8158b47c..2beb6dc74 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,9 @@ require ( github.com/aws/aws-lambda-go v1.30.0 github.com/aws/aws-sdk-go v1.43.45 github.com/paulvollmer/dependabot-config-go v0.1.1 - gopkg.in/yaml.v2 v2.4.0 + github.com/sirupsen/logrus v1.8.1 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b + gotest.tools v2.2.0+incompatible ) require ( @@ -21,6 +22,7 @@ require ( github.com/goccy/go-json v0.9.7 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.2 // indirect + github.com/google/go-cmp v0.5.7 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/lestrrat-go/backoff/v2 v2.0.8 // indirect github.com/lestrrat-go/blackmagic v1.0.0 // indirect @@ -32,13 +34,13 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.0.3-0.20211202183452-c5a74bcca799 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/sirupsen/logrus v1.8.1 // indirect golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f // indirect golang.org/x/net v0.0.0-20220421235706-1d1ef9303861 // indirect golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect golang.org/x/sys v0.0.0-20220422013727-9388b58f7150 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.28.0 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect ) require ( @@ -47,7 +49,7 @@ require ( github.com/golang-jwt/jwt v3.2.2+incompatible github.com/google/go-containerregistry v0.8.0 github.com/google/go-github/v40 v40.0.0 - github.com/jarcoal/httpmock v1.1.0 + github.com/jarcoal/httpmock v1.4.0 github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/lestrrat-go/jwx v1.2.25 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 diff --git a/go.sum b/go.sum index 49a46a91a..32324feb0 100644 --- a/go.sum +++ b/go.sum @@ -850,8 +850,8 @@ github.com/j-keck/arping v0.0.0-20160618110441-2cf9dc699c56/go.mod h1:ymszkNOg6t github.com/j-keck/arping v1.0.2/go.mod h1:aJbELhR92bSk7tp79AWM/ftfc90EfEi2bQJrbBFOsPw= github.com/jaguilar/vt100 v0.0.0-20150826170717-2703a27b14ea/go.mod h1:QMdK4dGB3YhEW2BmA1wgGpPYI3HZy/5gD705PXKUVSg= github.com/jarcoal/httpmock v1.0.5/go.mod h1:ATjnClrvW/3tijVmpL/va5Z3aAyGvqU3gCT8nX0Txik= -github.com/jarcoal/httpmock v1.1.0 h1:F47ChZj1Y2zFsCXxNkBPwNNKnAyOATcdQibk0qEdVCE= -github.com/jarcoal/httpmock v1.1.0/go.mod h1:ATjnClrvW/3tijVmpL/va5Z3aAyGvqU3gCT8nX0Txik= +github.com/jarcoal/httpmock v1.4.0 h1:BvhqnH0JAYbNudL2GMJKgOHe2CtKlzJ/5rWKyp+hc2k= +github.com/jarcoal/httpmock v1.4.0/go.mod h1:ftW1xULwo+j0R0JJkJIIi7UKigZUXCLLanykgjwBXL0= github.com/jellevandenhooff/dkim v0.0.0-20150330215556-f50fe3d243e1/go.mod h1:E0B/fFc00Y+Rasa88328GlI/XbtyysCtTHZS8h7IrBU= github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a/go.mod h1:xRskid8CManxVta/ALEhJha/pweKBaVG6fWgc0yH25s= github.com/jirfag/go-printf-func-name v0.0.0-20191110105641-45db9963cdd3/go.mod h1:HEWGJkRDzjJY2sqdDwxccsGicWEf9BQOZsq2tV+xzM0= @@ -973,6 +973,8 @@ github.com/mattn/go-zglob v0.0.1/go.mod h1:9fxibJccNxU2cnpIKLRRFA7zX7qhkJIQWBb44 github.com/mattn/goveralls v0.0.2/go.mod h1:8d1ZMHsd7fW6IRPKQh46F2WRpyib5/X4FOpevwGNQEw= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= +github.com/maxatome/go-testdeep v1.14.0 h1:rRlLv1+kI8eOI3OaBXZwb3O7xY3exRzdW5QyX48g9wI= +github.com/maxatome/go-testdeep v1.14.0/go.mod h1:lPZc/HAcJMP92l7yI6TRz1aZN5URwUBUAfUNvrclaNM= github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2/go.mod h1:eD9eIE7cdwcMi9rYluz88Jz2VyhSmden33/aXg4oVIY= github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= diff --git a/remediation/workflow/maintainedactions/doubts.txt b/remediation/workflow/maintainedactions/doubts.txt new file mode 100644 index 000000000..24b9546d4 --- /dev/null +++ b/remediation/workflow/maintainedactions/doubts.txt @@ -0,0 +1,6 @@ +1. How to handle the 'with' parameters in steps - should they be preserved when replacing actions? +2. How to handle version numbers - should we preserve the exact version or update to latest? +3. How to maintain the mapping between original and maintained actions: + - Should we load the JSON file every time or cache it? + - How to handle updates to the maintained actions list? + - How to ensure the UI stays in sync with the JSON file? diff --git a/remediation/workflow/maintainedactions/getlatestrelease.go b/remediation/workflow/maintainedactions/getlatestrelease.go new file mode 100644 index 000000000..3861d49e5 --- /dev/null +++ b/remediation/workflow/maintainedactions/getlatestrelease.go @@ -0,0 +1,40 @@ +package maintainedactions + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" +) + +type Release struct { + TagName string `json:"tag_name"` +} + +func GetLatestRelease(ownerRepo string) (string, error) { + // Build the URL dynamically and add `/actions` at the end + url := fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", ownerRepo) + fmt.Println("url ", url) + + resp, err := http.Get(url) + if err != nil { + return "", fmt.Errorf("error fetching release: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != 200 { + return "", fmt.Errorf("non-200 response: %s", resp.Status) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("error reading response: %w", err) + } + + var release Release + if err := json.Unmarshal(body, &release); err != nil { + return "", fmt.Errorf("error parsing JSON: %w", err) + } + + return release.TagName, nil +} diff --git a/remediation/workflow/maintainedactions/maintainedActions.go b/remediation/workflow/maintainedactions/maintainedActions.go new file mode 100644 index 000000000..e9ad132c1 --- /dev/null +++ b/remediation/workflow/maintainedactions/maintainedActions.go @@ -0,0 +1,150 @@ +package maintainedactions + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "path/filepath" + "strings" + + "github.com/step-security/secure-repo/remediation/workflow/metadata" + "github.com/step-security/secure-repo/remediation/workflow/permissions" + "gopkg.in/yaml.v3" +) + +// Action represents a GitHub Action in the maintained actions list +type Action struct { + Name string `json:"name"` + Description string `json:"description"` + ForkedFrom struct { + Name string `json:"name"` + } `json:"forkedFrom"` + Score int `json:"score"` + Image string `json:"image"` +} + +type replacement struct { + jobName string + stepIdx int + newAction string + originalAction string + latestVersion string +} + +// LoadMaintainedActions loads the maintained actions from the JSON file +func LoadMaintainedActions() (map[string]string, error) { + // Read the JSON file + jsonPath := filepath.Join("maintainedactions", "maintainedActions.json") + + data, err := ioutil.ReadFile(jsonPath) + if err != nil { + return nil, fmt.Errorf("failed to read maintained actions file: %v", err) + } + + // Parse the JSON + var actions []Action + if err := json.Unmarshal(data, &actions); err != nil { + return nil, fmt.Errorf("failed to parse maintained actions JSON: %v", err) + } + + // Create a map of original actions to their Step Security replacements + actionMap := make(map[string]string) + for _, action := range actions { + if action.ForkedFrom.Name != "" { + actionMap[action.ForkedFrom.Name] = action.Name + } + } + + return actionMap, nil +} + +// ReplaceActions replaces original actions with Step Security actions in a workflow +func ReplaceActions(inputYaml string) (string, bool, error) { + workflow := metadata.Workflow{} + updated := false + actionMap, err := LoadMaintainedActions() + if err != nil { + return "", updated, fmt.Errorf("unable to load maintained actions: %v", err) + } + err = yaml.Unmarshal([]byte(inputYaml), &workflow) + if err != nil { + return "", updated, fmt.Errorf("unable to parse yaml: %v", err) + } + + // Step 1: Check if anything needs to be replaced + + var replacements []replacement + + for jobName, job := range workflow.Jobs { + if metadata.IsCallingReusableWorkflow(job) { + continue + } + for stepIdx, step := range job.Steps { + // fmt.Println("step ", step.Uses) + actionName := strings.Split(step.Uses, "@")[0] + if newAction, ok := actionMap[actionName]; ok { + latestVersion, err := GetLatestRelease(newAction) + if err != nil { + return "", updated, fmt.Errorf("unable to get latest release: %v", err) + } + replacements = append(replacements, replacement{ + jobName: jobName, + stepIdx: stepIdx, + newAction: newAction, + originalAction: step.Uses, + latestVersion: latestVersion, + }) + } + } + } + if len(replacements) == 0 { + // No changes needed + return inputYaml, false, nil + } + + // Step 2: Now modify the YAML lines manually + t := yaml.Node{} + err = yaml.Unmarshal([]byte(inputYaml), &t) + if err != nil { + return "", updated, fmt.Errorf("unable to parse yaml: %v", err) + } + + inputLines := strings.Split(inputYaml, "\n") + inputLines, updated, err = replaceAction(&t, inputLines, replacements, updated) + if err != nil { + return "", updated, fmt.Errorf("unable to replace action: %v", err) + } + + output := strings.Join(inputLines, "\n") + + return output, updated, nil +} + +func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement, updated bool) ([]string, bool, error) { + 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) + if stepsNode == nil { + continue + } + + // Now get the specific step + stepNode := stepsNode.Content[r.stepIdx] + usesNode := permissions.IterateNode(stepNode, "uses", "!!str", 0) + if usesNode == nil { + continue + } + + lineNum := usesNode.Line - 1 // 0-based indexing + columnNum := usesNode.Column - 1 + + // Replace the line + oldLine := inputLines[lineNum] + prefix := oldLine[:columnNum] + inputLines[lineNum] = prefix + r.newAction + "@" + r.latestVersion + updated = true + + } + return inputLines, updated, nil +} diff --git a/remediation/workflow/maintainedactions/maintainedActions.json b/remediation/workflow/maintainedactions/maintainedActions.json new file mode 100644 index 000000000..273ce0616 --- /dev/null +++ b/remediation/workflow/maintainedactions/maintainedActions.json @@ -0,0 +1,497 @@ +[ + { + "name": "step-security/action-semantic-pull-request", + "description": "A GitHub Action that ensures that your PR title matches the Conventional Commits spec.", + "forkedFrom": { + "name": "amannn/action-semantic-pull-request" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/skip-duplicate-actions", + "description": "Save time and cost when using GitHub Actions", + "forkedFrom": { + "name": "fkirc/skip-duplicate-actions" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/git-restore-mtime-action", + "description": "A GitHub Workflow Action which restores timestamps of files in the current tree", + "forkedFrom": { + "name": "chetan/git-restore-mtime-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/dynamodb-actions", + "description": "Integrate GitHub Action with Amazon DynamoDB", + "forkedFrom": { + "name": "mooyoul/dynamodb-actions" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/publish-unit-test-result-action", + "description": "GitHub Action to publish unit test results on GitHub", + "forkedFrom": { + "name": "EnricoMi/publish-unit-test-result-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/setup-yq", + "description": "Sets up YQ, yet-another-markup-language-query-er, for use in your GitHub Actions workflow", + "forkedFrom": { + "name": "chrisdickinson/setup-yq" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/paths-filter", + "description": "Conditionally run actions based on files modified by PR, feature branch or pushed commits", + "forkedFrom": { + "name": "dorny/paths-filter" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/create-json", + "description": "GitHub Action to create a .json file to use in other steps of the workflow", + "forkedFrom": { + "name": "jsdaniell/create-json" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/npm-get-version-action", + "description": "This Action scans for a package.json file and reads the version number from that", + "forkedFrom": { + "name": "martinbeentjes/npm-get-version-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/change-string-case-action", + "description": "GitHub Action: Make a string lowercase, uppercase, or capitalized", + "forkedFrom": { + "name": "ASzc/change-string-case-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/ghaction-import-gpg", + "description": "GitHub Action to import a GPG key", + "forkedFrom": { + "name": "crazy-max/ghaction-import-gpg" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/conventional-pr-title-action", + "description": "Ensure your PR title matches the Conventional Commits spec", + "forkedFrom": { + "name": "aslafy-z/conventional-pr-title-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/semver-utils", + "description": "One-stop shop for working with semantic versions in your GitHub Actions workflows", + "forkedFrom": { + "name": "madhead/semver-utils" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/pr-labeler-action", + "description": "Automatically labels your PRs based on branch name patterns like feature/* or fix/*", + "forkedFrom": { + "name": "TimonVS/pr-labeler-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/workflow-dispatch", + "description": "A GitHub Action for triggering workflows, using the `workflow_dispatch` event", + "forkedFrom": { + "name": "benc-uk/workflow-dispatch" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/retry", + "description": "Retries a GitHub Action step on failure or timeout", + "forkedFrom": { + "name": "nick-fields/retry" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/action-send-mail", + "description": "A GitHub Action to send an email to multiple recipients", + "forkedFrom": { + "name": "dawidd6/action-send-mail" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/helm-gh-pages", + "description": "A GitHub Action for publishing Helm charts to GitHub Pages", + "forkedFrom": { + "name": "stefanprodan/helm-gh-pages" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/ghaction-setup-docker", + "description": "GitHub Action to set up (download and install) Docker CE", + "forkedFrom": { + "name": "crazy-max/ghaction-setup-docker" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/rust-cache", + "description": "A GitHub Action that implements smart caching for rust/cargo projects", + "forkedFrom": { + "name": "Swatinem/rust-cache" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/nats-action", + "description": "start nats server(s) for GitHub Actions", + "forkedFrom": { + "name": "onichandame/nats-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/foundry-toolchain", + "description": "GitHub action to install Foundry", + "forkedFrom": { + "name": "foundry-rs/foundry-toolchain" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/gh-docker-logs", + "description": "GitHub Action to collect logs from all docker containers", + "forkedFrom": { + "name": "jwalton/gh-docker-logs" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/gh-actions-lua", + "description": "GitHub action for Lua/LuaJIT", + "forkedFrom": { + "name": "leafo/gh-actions-lua" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/vitest-coverage-report-action", + "description": "A GitHub Action to report vitest test coverage results", + "forkedFrom": { + "name": "davelosert/vitest-coverage-report-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/release-notes-generator-action", + "description": "Action to auto generate a release note based on your events", + "forkedFrom": { + "name": "Decathlon/release-notes-generator-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/action-cond", + "description": "Conditional value for GitHub Action - missing expression for GitHub Actions", + "forkedFrom": { + "name": "haya14busa/action-cond" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/action-discord", + "description": "GitHub Action that sends a Discord message.", + "forkedFrom": { + "name": "Ilshidur/action-discord" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/workflow-conclusion-action", + "description": "GitHub action to get workflow conclusion.", + "forkedFrom": { + "name": "technote-space/workflow-conclusion-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/secrets-sync-action", + "description": "A GitHub Action that can sync secrets from one repository to many others.", + "forkedFrom": { + "name": "jpoehnelt/secrets-sync-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/jest-coverage-report-action", + "description": "GitHub action to track your code coverage in every pull request", + "forkedFrom": { + "name": "ArtiomTr/jest-coverage-report-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/google-github-auth", + "description": "A GitHub Action for authenticating to Google Cloud", + "forkedFrom": { + "name": "google-github-actions/auth" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/increment", + "description": "A GitHub Action to increment a repository variable.", + "forkedFrom": { + "name": "action-pack/increment" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/actions-find-and-replace-string", + "description": "A GitHub action to execute find-and-replace on strings", + "forkedFrom": { + "name": "mad9000/actions-find-and-replace-string" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/assign-author", + "description": "GitHub Actions to assign author to issue or PR", + "forkedFrom": { + "name": "technote-space/assign-author" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/setup-gh-cli-action", + "description": "A GitHub action that installs or updates the gh CLI", + "forkedFrom": { + "name": "sersoft-gmbh/setup-gh-cli-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/close-milestone", + "description": "A GitHub action to remove a milestone by the milestone's name", + "forkedFrom": { + "name": "Akkjon/close-milestone" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/ssh-key-action", + "description": "GitHub Action that installs SSH key to .ssh", + "forkedFrom": { + "name": "shimataro/ssh-key-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/actions-hugo", + "description": "GitHub Actions for Hugo ⚡️ Setup Hugo quickly and build your site fast. Hugo extended, Hugo Modules, Linux (Ubuntu), macOS, and Windows are supported.", + "forkedFrom": { + "name": "peaceiris/actions-hugo" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/mongodb-github-action", + "description": "Use MongoDB in GitHub Actions", + "forkedFrom": { + "name": "supercharge/mongodb-github-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/action-setup", + "description": "Install pnpm package manager", + "forkedFrom": { + "name": "pnpm/action-setup" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/setup-zig", + "description": "A GitHub action to install a Zig compiler for usage in GitHub Actions workflow", + "forkedFrom": { + "name": "mlugg/setup-zig" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/run-vcpkg", + "description": "A GitHub Action to setup vcpkg for C++ based projects", + "forkedFrom": { + "name": "lukka/run-vcpkg" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/changed-files", + "description": "GitHub action to retrieve all (added, copied, modified, deleted, renamed, type changed, unmerged, unknown) files and directories.", + "forkedFrom": { + "name": "tj-actions/changed-files" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/reviewdog-action-setup", + "description": "Setup reviewdog action", + "forkedFrom": { + "name": "reviewdog/action-setup" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/action-gh-release", + "description": "GitHub Action for creating GitHub Releases", + "forkedFrom": { + "name": "softprops/action-gh-release" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/github-actions-slack", + "description": "GitHub Action for sending message to Slack - With support for Slack's optional arguments", + "forkedFrom": { + "name": "archive/github-actions-slack" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/envsubst-action", + "description": "GitHub Action for envsubst", + "forkedFrom": { + "name": "danielr1996/envsubst-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/test-reporter", + "description": "Displays test results from popular testing frameworks directly in GitHub", + "forkedFrom": { + "name": "dorny/test-reporter" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/github-action-get-latest-release", + "description": "A GitHub action to get the latest release from another repository", + "forkedFrom": { + "name": "pozetroninc/github-action-get-latest-release" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/runs-on-cache", + "description": "Shockingly faster GitHub Action cache with S3 backend", + "forkedFrom": { + "name": "runs-on/cache" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/multi-labeler", + "description": "Multi labeler for title, body, comments, commit messages, branch, author or files with automated status checks", + "forkedFrom": { + "name": "fuxingloh/multi-labeler" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/background-action", + "description": "Background commands with log tailing/capture; waits until file/port/socket/http are ready to proceed. Isolates/dedupe errors", + "forkedFrom": { + "name": "JarvusInnovations/background-action" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/add-pr-comment", + "description": "GitHub Action which adds a comment to a pull request's issue", + "forkedFrom": { + "name": "mshick/add-pr-comment" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + }, + { + "name": "step-security/ghaction-setup-containerd", + "description": "GitHub Action to set up containerd", + "forkedFrom": { + "name": "crazy-max/ghaction-setup-containerd" + }, + "score": 10, + "image": "https://avatars.githubusercontent.com/u/88700172?v=4" + } +] \ No newline at end of file diff --git a/remediation/workflow/maintainedactions/maintainedactions_test.go b/remediation/workflow/maintainedactions/maintainedactions_test.go new file mode 100644 index 000000000..4788baf24 --- /dev/null +++ b/remediation/workflow/maintainedactions/maintainedactions_test.go @@ -0,0 +1,108 @@ +package maintainedactions + +import ( + "io/ioutil" + "path" + "testing" + + "github.com/jarcoal/httpmock" +) + +func TestReplaceActions(t *testing.T) { + const inputDirectory = "../../../testfiles/maintainedactions/input" + const outputDirectory = "../../../testfiles/maintainedactions/output" + + // Activate httpmock + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + // Mock GitHub API responses for getting latest releases + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/action-semantic-pull-request/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v5.5.5", + "name": "v5.5.5", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/skip-duplicate-actions/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v5.3.2", + "name": "v5.3.2", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/git-restore-mtime-action/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v2.1.0", + "name": "v2.1.0", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + tests := []struct { + name string + inputFile string + outputFile string + wantUpdated bool + wantErr bool + }{ + { + name: "one job with actions to replace", + inputFile: "oneJob.yml", + outputFile: "oneJob.yml", + wantUpdated: true, + wantErr: false, + }, + { + name: "no changes needed - already using maintained actions", + inputFile: "noChangesNeeded.yml", + outputFile: "noChangesNeeded.yml", + wantUpdated: false, + wantErr: false, + }, + { + name: "double job with actions to replace", + inputFile: "doubleJob.yml", + outputFile: "doubleJob.yml", + wantUpdated: true, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Read input file + input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.inputFile)) + if err != nil { + t.Fatalf("error reading input file: %v", err) + } + + // Call ReplaceActions + got, updated, err := ReplaceActions(string(input)) + + // Check error + if (err != nil) != tt.wantErr { + t.Errorf("ReplaceActions() error = %v, wantErr %v", err, tt.wantErr) + return + } + + // Check if updated flag matches + if updated != tt.wantUpdated { + t.Errorf("ReplaceActions() updated = %v, wantUpdated %v", updated, tt.wantUpdated) + } + + // Read expected output file + expectedOutput, err := ioutil.ReadFile(path.Join(outputDirectory, tt.outputFile)) + if err != nil { + t.Fatalf("error reading expected output file: %v", err) + } + + // Compare output with expected + if got != string(expectedOutput) { + t.Errorf("ReplaceActions() = %v, want %v", got, string(expectedOutput)) + } + }) + } +} diff --git a/remediation/workflow/permissions/permissions.go b/remediation/workflow/permissions/permissions.go index 8ffa5d7ab..5964c0c1f 100644 --- a/remediation/workflow/permissions/permissions.go +++ b/remediation/workflow/permissions/permissions.go @@ -12,18 +12,19 @@ import ( ) type SecureWorkflowReponse struct { - OriginalInput string - FinalOutput string - IsChanged bool - HasErrors bool - AlreadyHasPermissions bool - PinnedActions bool - AddedHardenRunner bool - AddedPermissions bool - IncorrectYaml bool - WorkflowFetchError bool - JobErrors []JobError - MissingActions []string + OriginalInput string + FinalOutput string + IsChanged bool + HasErrors bool + AlreadyHasPermissions bool + AddedMaintainedActions bool + PinnedActions bool + AddedHardenRunner bool + AddedPermissions bool + IncorrectYaml bool + WorkflowFetchError bool + JobErrors []JobError + MissingActions []string } type JobError struct { diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 03656f46e..1015269e2 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -3,6 +3,7 @@ package workflow import ( "github.com/aws/aws-sdk-go/service/dynamodb/dynamodbiface" "github.com/step-security/secure-repo/remediation/workflow/hardenrunner" + "github.com/step-security/secure-repo/remediation/workflow/maintainedactions" "github.com/step-security/secure-repo/remediation/workflow/permissions" "github.com/step-security/secure-repo/remediation/workflow/pin" ) @@ -14,8 +15,8 @@ const ( ) func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc dynamodbiface.DynamoDBAPI, params ...interface{}) (*permissions.SecureWorkflowReponse, error) { - pinActions, addHardenRunner, addPermissions, addProjectComment := true, true, true, true - pinnedActions, addedHardenRunner, addedPermissions := false, false, false + pinActions, addHardenRunner, addPermissions, addProjectComment, addMaintainedActions := true, true, true, true, true + pinnedActions, addedHardenRunner, addedPermissions, addedMaintainedActions := false, false, false, false ignoreMissingKBs := false exemptedActions, pinToImmutable := []string{}, false if len(params) > 0 { @@ -77,6 +78,13 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d addedPermissions = !secureWorkflowReponse.HasErrors } + if addMaintainedActions { + secureWorkflowReponse.FinalOutput, addedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput) + if err != nil { + secureWorkflowReponse.HasErrors = true + } + } + if pinActions { pinnedAction, pinnedDocker := false, false secureWorkflowReponse.FinalOutput, pinnedAction, _ = pin.PinActions(secureWorkflowReponse.FinalOutput, exemptedActions, pinToImmutable) @@ -97,5 +105,6 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d secureWorkflowReponse.PinnedActions = pinnedActions secureWorkflowReponse.AddedHardenRunner = addedHardenRunner secureWorkflowReponse.AddedPermissions = addedPermissions + secureWorkflowReponse.AddedMaintainedActions = addedMaintainedActions return secureWorkflowReponse, nil } diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index 9b5baa8b7..8da995c1f 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -1,6 +1,7 @@ package workflow import ( + "fmt" "io/ioutil" "log" "os" @@ -10,6 +11,27 @@ import ( "github.com/jarcoal/httpmock" ) +func compareStrings(a, b string) { + maxLen := len(a) + if len(b) > maxLen { + maxLen = len(b) + } + + for i := 0; i < maxLen; i++ { + var ca, cb byte = '-', '-' + if i < len(a) { + ca = a[i] + } + if i < len(b) { + cb = b[i] + } + if ca != cb { + fmt.Printf("Mismatch at byte %d: expected '%c' (0x%x), got '%c' (0x%x)\n", + i, ca, ca, cb, cb) + } + } +} + func TestSecureWorkflow(t *testing.T) { const inputDirectory = "../../testfiles/secureworkflow/input" const outputDirectory = "../../testfiles/secureworkflow/output" @@ -107,12 +129,46 @@ func TestSecureWorkflow(t *testing.T) { ]`), ) + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/action-semantic-pull-request/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v5.5.5", + "name": "v5.5.5", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/git-restore-mtime-action/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v2.1.0", + "name": "v2.1.0", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/github/super-linter/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v4.9.0", + "name": "v4.9.0", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/skip-duplicate-actions/releases/latest", + httpmock.NewStringResponder(200, `{ + "tag_name": "v2.1.0", + "name": "v2.1.0", + "body": "Release notes", + "created_at": "2023-01-01T00:00:00Z" + }`)) + tests := []struct { - fileName string - wantPinnedActions bool - wantAddedHardenRunner bool - wantAddedPermissions bool + fileName string + wantPinnedActions bool + wantAddedHardenRunner bool + wantAddedPermissions bool + wantAddedMaintainedActions bool }{ + {fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true}, {fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true}, {fileName: "missingaction.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false}, {fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true}, @@ -145,6 +201,11 @@ func TestSecureWorkflow(t *testing.T) { case "multiplejobperms.yml": queryParams["addHardenRunner"] = "false" queryParams["pinActions"] = "false" + case "oneJob.yml": + queryParams["addMaintainedActions"] = "true" + queryParams["addHardenRunner"] = "true" + queryParams["pinActions"] = "true" + queryParams["addPermissions"] = "false" } queryParams["addProjectComment"] = "false" @@ -159,7 +220,6 @@ func TestSecureWorkflow(t *testing.T) { if err != nil { log.Fatal(err) } - if output.FinalOutput != string(expectedOutput) { t.Errorf("test failed %s did not match expected output\n%s", test.fileName, output.FinalOutput) } diff --git a/testfiles/maintainedActions/input/doubleJob.yml b/testfiles/maintainedActions/input/doubleJob.yml new file mode 100644 index 000000000..e9ed7d912 --- /dev/null +++ b/testfiles/maintainedActions/input/doubleJob.yml @@ -0,0 +1,31 @@ +name: Test Workflow - Double Job +on: push + +jobs: + test: + runs-on: ubuntu-latest + 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: step-security/git-restore-mtime-action@v2.1.0 + with: + pattern: '**/*' + + build: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: '16' + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- \ No newline at end of file diff --git a/testfiles/maintainedActions/input/noChangesNeeded.yml b/testfiles/maintainedActions/input/noChangesNeeded.yml new file mode 100644 index 000000000..5557b01e6 --- /dev/null +++ b/testfiles/maintainedActions/input/noChangesNeeded.yml @@ -0,0 +1,17 @@ +name: Test Workflow - No Changes Needed +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: step-security/checkout@v3 + - uses: step-security/action-semantic-pull-request@v5.5.5 + with: + types: feat,fix,chore + - uses: step-security/skip-duplicate-actions@v5.3.2 + with: + do_not_skip: '["release"]' + - uses: step-security/git-restore-mtime-action@v2.1.0 + with: + pattern: '**/*' \ No newline at end of file diff --git a/testfiles/maintainedActions/output/doubleJob.yml b/testfiles/maintainedActions/output/doubleJob.yml new file mode 100644 index 000000000..a956859e2 --- /dev/null +++ b/testfiles/maintainedActions/output/doubleJob.yml @@ -0,0 +1,31 @@ +name: Test Workflow - Double Job +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: step-security/action-semantic-pull-request@v5.5.5 + with: + types: feat,fix,chore + - uses: step-security/skip-duplicate-actions@v5.3.2 + with: + do_not_skip: '["release"]' + - uses: step-security/git-restore-mtime-action@v2.1.0 + with: + pattern: '**/*' + + build: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: '16' + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- \ No newline at end of file diff --git a/testfiles/maintainedActions/output/noChangesNeeded.yml b/testfiles/maintainedActions/output/noChangesNeeded.yml new file mode 100644 index 000000000..5557b01e6 --- /dev/null +++ b/testfiles/maintainedActions/output/noChangesNeeded.yml @@ -0,0 +1,17 @@ +name: Test Workflow - No Changes Needed +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: step-security/checkout@v3 + - uses: step-security/action-semantic-pull-request@v5.5.5 + with: + types: feat,fix,chore + - uses: step-security/skip-duplicate-actions@v5.3.2 + with: + do_not_skip: '["release"]' + - uses: step-security/git-restore-mtime-action@v2.1.0 + with: + pattern: '**/*' \ No newline at end of file diff --git a/testfiles/secureworkflow/input/oneJob.yml b/testfiles/secureworkflow/input/oneJob.yml new file mode 100644 index 000000000..fb65d3a4e --- /dev/null +++ b/testfiles/secureworkflow/input/oneJob.yml @@ -0,0 +1,26 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - 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: '**/*' + + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - uses: github/super-linter@v3 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DISABLE_ERRORS: true \ No newline at end of file diff --git a/testfiles/secureworkflow/output/oneJob.yml b/testfiles/secureworkflow/output/oneJob.yml new file mode 100644 index 000000000..8a43fd7ac --- /dev/null +++ b/testfiles/secureworkflow/output/oneJob.yml @@ -0,0 +1,36 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 + with: + egress-policy: audit + + - uses: actions/checkout@v3 + - uses: step-security/action-semantic-pull-request@v5.5.5 + with: + types: feat,fix,chore + - uses: step-security/skip-duplicate-actions@v2.1.0 + with: + do_not_skip: '["release"]' + - uses: step-security/git-restore-mtime-action@v2.1.0 + with: + pattern: '**/*' + + lint: + runs-on: ubuntu-latest + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 + with: + egress-policy: audit + + - uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0 + - uses: github/super-linter@34b2f8032d759425f6b42ea2e52231b33ae05401 # v3.17.1 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DISABLE_ERRORS: true \ No newline at end of file From 28d68b908ce25c10f3c710fd04f22423c2e73a41 Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Tue, 29 Apr 2025 22:32:36 +0530 Subject: [PATCH 2/6] del uneccessary stuff --- .../workflow/maintainedactions/doubts.txt | 6 ----- remediation/workflow/secureworkflow_test.go | 22 ------------------- 2 files changed, 28 deletions(-) delete mode 100644 remediation/workflow/maintainedactions/doubts.txt diff --git a/remediation/workflow/maintainedactions/doubts.txt b/remediation/workflow/maintainedactions/doubts.txt deleted file mode 100644 index 24b9546d4..000000000 --- a/remediation/workflow/maintainedactions/doubts.txt +++ /dev/null @@ -1,6 +0,0 @@ -1. How to handle the 'with' parameters in steps - should they be preserved when replacing actions? -2. How to handle version numbers - should we preserve the exact version or update to latest? -3. How to maintain the mapping between original and maintained actions: - - Should we load the JSON file every time or cache it? - - How to handle updates to the maintained actions list? - - How to ensure the UI stays in sync with the JSON file? diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index 8da995c1f..9ce1c95af 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -1,7 +1,6 @@ package workflow import ( - "fmt" "io/ioutil" "log" "os" @@ -11,27 +10,6 @@ import ( "github.com/jarcoal/httpmock" ) -func compareStrings(a, b string) { - maxLen := len(a) - if len(b) > maxLen { - maxLen = len(b) - } - - for i := 0; i < maxLen; i++ { - var ca, cb byte = '-', '-' - if i < len(a) { - ca = a[i] - } - if i < len(b) { - cb = b[i] - } - if ca != cb { - fmt.Printf("Mismatch at byte %d: expected '%c' (0x%x), got '%c' (0x%x)\n", - i, ca, ca, cb, cb) - } - } -} - func TestSecureWorkflow(t *testing.T) { const inputDirectory = "../../testfiles/secureworkflow/input" const outputDirectory = "../../testfiles/secureworkflow/output" From 703e79479757062c4ec59dd5e3301135a6061e3f Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Thu, 1 May 2025 19:15:45 +0530 Subject: [PATCH 3/6] add change for tag --- .../maintainedactions/getlatestrelease.go | 68 +++++++++++++------ .../maintainedactions/maintainedActions.go | 11 ++- .../maintainedactions_test.go | 21 ++++++ remediation/workflow/secureworkflow_test.go | 48 ++++++++++++- .../maintainedActions/input/doubleJob.yml | 2 +- testfiles/maintainedActions/input/oneJob.yml | 17 +++++ .../maintainedActions/output/doubleJob.yml | 6 +- testfiles/maintainedActions/output/oneJob.yml | 17 +++++ testfiles/secureworkflow/input/oneJob.yml | 4 +- testfiles/secureworkflow/output/oneJob.yml | 10 +-- 10 files changed, 164 insertions(+), 40 deletions(-) create mode 100644 testfiles/maintainedActions/input/oneJob.yml create mode 100644 testfiles/maintainedActions/output/oneJob.yml diff --git a/remediation/workflow/maintainedactions/getlatestrelease.go b/remediation/workflow/maintainedactions/getlatestrelease.go index 3861d49e5..4ba0068eb 100644 --- a/remediation/workflow/maintainedactions/getlatestrelease.go +++ b/remediation/workflow/maintainedactions/getlatestrelease.go @@ -1,40 +1,66 @@ package maintainedactions import ( - "encoding/json" + "context" "fmt" - "io/ioutil" - "net/http" + "os" + "strings" + + "github.com/google/go-github/v40/github" + "golang.org/x/oauth2" ) type Release struct { TagName string `json:"tag_name"` } -func GetLatestRelease(ownerRepo string) (string, error) { - // Build the URL dynamically and add `/actions` at the end - url := fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", ownerRepo) - fmt.Println("url ", url) - - resp, err := http.Get(url) - if err != nil { - return "", fmt.Errorf("error fetching release: %w", err) +func getMajorVersion(version string) string { + hasVPrefix := strings.HasPrefix(version, "v") + version = strings.TrimPrefix(version, "v") + parts := strings.Split(version, ".") + if len(parts) > 0 { + if hasVPrefix { + return "v" + parts[0] + } + return parts[0] + } + if hasVPrefix { + return "v" + version } - defer resp.Body.Close() + return version +} - if resp.StatusCode != 200 { - return "", fmt.Errorf("non-200 response: %s", resp.Status) +func GetLatestRelease(ownerRepo string) (string, error) { + splitOnSlash := strings.Split(ownerRepo, "/") + if len(splitOnSlash) != 2 { + return "", fmt.Errorf("invalid owner/repo format: %s", ownerRepo) } + owner := splitOnSlash[0] + repo := splitOnSlash[1] + + ctx := context.Background() - body, err := ioutil.ReadAll(resp.Body) + // First try without token + client := github.NewClient(nil) + release, _, err := client.Repositories.GetLatestRelease(ctx, owner, repo) if err != nil { - return "", fmt.Errorf("error reading response: %w", err) - } + // If failed, try with token + token := os.Getenv("PAT") + if token == "" { + return "", fmt.Errorf("failed to get latest release and no GITHUB_TOKEN available: %w", err) + } + + ts := oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ) + tc := oauth2.NewClient(ctx, ts) + client = github.NewClient(tc) - var release Release - if err := json.Unmarshal(body, &release); err != nil { - return "", fmt.Errorf("error parsing JSON: %w", err) + release, _, err = client.Repositories.GetLatestRelease(ctx, owner, repo) + if err != nil { + return "", fmt.Errorf("failed to get latest release with token: %w", err) + } } - return release.TagName, nil + return getMajorVersion(release.GetTagName()), nil } diff --git a/remediation/workflow/maintainedactions/maintainedActions.go b/remediation/workflow/maintainedactions/maintainedActions.go index e9ad132c1..bb411e881 100644 --- a/remediation/workflow/maintainedactions/maintainedActions.go +++ b/remediation/workflow/maintainedactions/maintainedActions.go @@ -35,7 +35,7 @@ type replacement struct { func LoadMaintainedActions() (map[string]string, error) { // Read the JSON file jsonPath := filepath.Join("maintainedactions", "maintainedActions.json") - + // jsonPath := filepath.Join("maintainedActions.json") data, err := ioutil.ReadFile(jsonPath) if err != nil { return nil, fmt.Errorf("failed to read maintained actions file: %v", err) @@ -110,17 +110,14 @@ func ReplaceActions(inputYaml string) (string, bool, error) { } inputLines := strings.Split(inputYaml, "\n") - inputLines, updated, err = replaceAction(&t, inputLines, replacements, updated) - if err != nil { - return "", updated, fmt.Errorf("unable to replace action: %v", err) - } + inputLines, updated = replaceAction(&t, inputLines, replacements, updated) output := strings.Join(inputLines, "\n") return output, updated, nil } -func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement, updated bool) ([]string, bool, error) { +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) @@ -146,5 +143,5 @@ func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement updated = true } - return inputLines, updated, nil + return inputLines, updated } diff --git a/remediation/workflow/maintainedactions/maintainedactions_test.go b/remediation/workflow/maintainedactions/maintainedactions_test.go index 4788baf24..62b50fd04 100644 --- a/remediation/workflow/maintainedactions/maintainedactions_test.go +++ b/remediation/workflow/maintainedactions/maintainedactions_test.go @@ -1,13 +1,33 @@ package maintainedactions import ( + "fmt" "io/ioutil" + "os" "path" "testing" "github.com/jarcoal/httpmock" ) +// WriteYAML writes the given string content to a YAML file with the specified filename. +func WriteYAML(filename string, content string) error { + // Create or truncate the file + file, err := os.Create(filename) + if err != nil { + return fmt.Errorf("failed to create file %s: %w", filename, err) + } + defer file.Close() + + // Write the string content to the file + _, err = file.WriteString(content) + if err != nil { + return fmt.Errorf("failed to write to file %s: %w", filename, err) + } + + return nil +} + func TestReplaceActions(t *testing.T) { const inputDirectory = "../../../testfiles/maintainedactions/input" const outputDirectory = "../../../testfiles/maintainedactions/output" @@ -101,6 +121,7 @@ func TestReplaceActions(t *testing.T) { // Compare output with expected if got != string(expectedOutput) { + WriteYAML(tt.outputFile+"second", got) t.Errorf("ReplaceActions() = %v, want %v", got, string(expectedOutput)) } }) diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index 9ce1c95af..aab090631 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -139,6 +139,51 @@ func TestSecureWorkflow(t *testing.T) { "created_at": "2023-01-01T00:00:00Z" }`)) + // Mock APIs for step-security/action-semantic-pull-request + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/action-semantic-pull-request/commits/v5", + httpmock.NewStringResponder(200, `a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/action-semantic-pull-request/git/matching-refs/tags/v5.", + httpmock.NewStringResponder(200, `[ + { + "ref": "refs/tags/v5.5.5", + "object": { + "sha": "a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0", + "type": "commit" + } + } + ]`)) + + // Mock APIs for step-security/skip-duplicate-actions + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/skip-duplicate-actions/commits/v2", + httpmock.NewStringResponder(200, `b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/skip-duplicate-actions/git/matching-refs/tags/v2.", + httpmock.NewStringResponder(200, `[ + { + "ref": "refs/tags/v2.1.0", + "object": { + "sha": "b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1", + "type": "commit" + } + } + ]`)) + + // Mock APIs for step-security/git-restore-mtime-action + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/git-restore-mtime-action/commits/v2", + httpmock.NewStringResponder(200, `c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1b2`)) + + httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/git-restore-mtime-action/git/matching-refs/tags/v2.", + httpmock.NewStringResponder(200, `[ + { + "ref": "refs/tags/v2.1.0", + "object": { + "sha": "c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1b2", + "type": "commit" + } + } + ]`)) + tests := []struct { fileName string wantPinnedActions bool @@ -198,6 +243,7 @@ func TestSecureWorkflow(t *testing.T) { if err != nil { log.Fatal(err) } + if output.FinalOutput != string(expectedOutput) { t.Errorf("test failed %s did not match expected output\n%s", test.fileName, output.FinalOutput) } @@ -213,6 +259,6 @@ func TestSecureWorkflow(t *testing.T) { if output.PinnedActions != test.wantPinnedActions { t.Errorf("test failed %s did not match expected PinnedActions value. Expected:%v Actual:%v", test.fileName, test.wantPinnedActions, output.PinnedActions) } - } + } } diff --git a/testfiles/maintainedActions/input/doubleJob.yml b/testfiles/maintainedActions/input/doubleJob.yml index e9ed7d912..9b4807e7c 100644 --- a/testfiles/maintainedActions/input/doubleJob.yml +++ b/testfiles/maintainedActions/input/doubleJob.yml @@ -11,7 +11,7 @@ jobs: - uses: fkirc/skip-duplicate-actions@v5 with: do_not_skip: '["release"]' - - uses: step-security/git-restore-mtime-action@v2.1.0 + - uses: step-security/git-restore-mtime-action@v2 with: pattern: '**/*' diff --git a/testfiles/maintainedActions/input/oneJob.yml b/testfiles/maintainedActions/input/oneJob.yml new file mode 100644 index 000000000..4a28c9e49 --- /dev/null +++ b/testfiles/maintainedActions/input/oneJob.yml @@ -0,0 +1,17 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - 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: '**/*' \ No newline at end of file diff --git a/testfiles/maintainedActions/output/doubleJob.yml b/testfiles/maintainedActions/output/doubleJob.yml index a956859e2..703b8c969 100644 --- a/testfiles/maintainedActions/output/doubleJob.yml +++ b/testfiles/maintainedActions/output/doubleJob.yml @@ -5,13 +5,13 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: step-security/action-semantic-pull-request@v5.5.5 + - uses: step-security/action-semantic-pull-request@v5 with: types: feat,fix,chore - - uses: step-security/skip-duplicate-actions@v5.3.2 + - uses: step-security/skip-duplicate-actions@v5 with: do_not_skip: '["release"]' - - uses: step-security/git-restore-mtime-action@v2.1.0 + - uses: step-security/git-restore-mtime-action@v2 with: pattern: '**/*' diff --git a/testfiles/maintainedActions/output/oneJob.yml b/testfiles/maintainedActions/output/oneJob.yml new file mode 100644 index 000000000..ce73bf408 --- /dev/null +++ b/testfiles/maintainedActions/output/oneJob.yml @@ -0,0 +1,17 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - 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: '**/*' \ No newline at end of file diff --git a/testfiles/secureworkflow/input/oneJob.yml b/testfiles/secureworkflow/input/oneJob.yml index fb65d3a4e..6201dd2c7 100644 --- a/testfiles/secureworkflow/input/oneJob.yml +++ b/testfiles/secureworkflow/input/oneJob.yml @@ -14,10 +14,10 @@ jobs: do_not_skip: '["release"]' - uses: chetan/git-restore-mtime-action@v1 with: - pattern: '**/*' + pattern: '**/*' lint: - runs-on: ubuntu-latest + runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - uses: github/super-linter@v3 diff --git a/testfiles/secureworkflow/output/oneJob.yml b/testfiles/secureworkflow/output/oneJob.yml index 8a43fd7ac..91ec1376d 100644 --- a/testfiles/secureworkflow/output/oneJob.yml +++ b/testfiles/secureworkflow/output/oneJob.yml @@ -11,18 +11,18 @@ jobs: egress-policy: audit - uses: actions/checkout@v3 - - uses: step-security/action-semantic-pull-request@v5.5.5 + - uses: step-security/action-semantic-pull-request@a1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0 # v5.5.5 with: types: feat,fix,chore - - uses: step-security/skip-duplicate-actions@v2.1.0 + - uses: step-security/skip-duplicate-actions@b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1 # v2.1.0 with: do_not_skip: '["release"]' - - uses: step-security/git-restore-mtime-action@v2.1.0 + - uses: step-security/git-restore-mtime-action@c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0a1b2 # v2.1.0 with: - pattern: '**/*' + pattern: '**/*' lint: - runs-on: ubuntu-latest + runs-on: ubuntu-latest steps: - name: Harden the runner (Audit all outbound calls) uses: step-security/harden-runner@ebacdc22ef6c2cfb85ee5ded8f2e640f4c776dd5 # v2.0.0 From fc469a13e60fa09df47fbd647461d24bd89b8245 Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Fri, 2 May 2025 13:12:55 +0530 Subject: [PATCH 4/6] add changes for exempted actions --- .../maintainedactions/maintainedActions.go | 14 ++++++- .../maintainedactions_test.go | 41 ++++++++----------- remediation/workflow/secureworkflow.go | 9 +++- .../input/exemtedMaintainedActions.yml | 35 ++++++++++++++++ .../output/exemtedMaintainedActions.yml | 35 ++++++++++++++++ 5 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 testfiles/maintainedActions/input/exemtedMaintainedActions.yml create mode 100644 testfiles/maintainedActions/output/exemtedMaintainedActions.yml diff --git a/remediation/workflow/maintainedactions/maintainedActions.go b/remediation/workflow/maintainedactions/maintainedActions.go index bb411e881..feb867657 100644 --- a/remediation/workflow/maintainedactions/maintainedActions.go +++ b/remediation/workflow/maintainedactions/maintainedActions.go @@ -59,7 +59,7 @@ func LoadMaintainedActions() (map[string]string, error) { } // ReplaceActions replaces original actions with Step Security actions in a workflow -func ReplaceActions(inputYaml string) (string, bool, error) { +func ReplaceActions(inputYaml string, customerMaintainedActions []string) (string, bool, error) { workflow := metadata.Workflow{} updated := false actionMap, err := LoadMaintainedActions() @@ -83,6 +83,9 @@ func ReplaceActions(inputYaml string) (string, bool, error) { // fmt.Println("step ", step.Uses) actionName := strings.Split(step.Uses, "@")[0] if newAction, ok := actionMap[actionName]; ok { + if isMaintained(newAction, customerMaintainedActions) { + continue + } latestVersion, err := GetLatestRelease(newAction) if err != nil { return "", updated, fmt.Errorf("unable to get latest release: %v", err) @@ -145,3 +148,12 @@ func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement } return inputLines, updated } + +func isMaintained(actionName string, maintainedActions []string) bool { + for _, maintainedAction := range maintainedActions { + if maintainedAction == actionName { + return true + } + } + return false +} diff --git a/remediation/workflow/maintainedactions/maintainedactions_test.go b/remediation/workflow/maintainedactions/maintainedactions_test.go index 62b50fd04..2e2a43281 100644 --- a/remediation/workflow/maintainedactions/maintainedactions_test.go +++ b/remediation/workflow/maintainedactions/maintainedactions_test.go @@ -1,33 +1,13 @@ package maintainedactions import ( - "fmt" "io/ioutil" - "os" "path" "testing" "github.com/jarcoal/httpmock" ) -// WriteYAML writes the given string content to a YAML file with the specified filename. -func WriteYAML(filename string, content string) error { - // Create or truncate the file - file, err := os.Create(filename) - if err != nil { - return fmt.Errorf("failed to create file %s: %w", filename, err) - } - defer file.Close() - - // Write the string content to the file - _, err = file.WriteString(content) - if err != nil { - return fmt.Errorf("failed to write to file %s: %w", filename, err) - } - - return nil -} - func TestReplaceActions(t *testing.T) { const inputDirectory = "../../../testfiles/maintainedactions/input" const outputDirectory = "../../../testfiles/maintainedactions/output" @@ -89,6 +69,12 @@ func TestReplaceActions(t *testing.T) { wantUpdated: true, wantErr: false, }, + { + name: "exemtedMaintainedActions.yml", + inputFile: "exemtedMaintainedActions.yml", + outputFile: "exemtedMaintainedActions.yml", + wantUpdated: true, + }, } for _, tt := range tests { @@ -100,11 +86,18 @@ func TestReplaceActions(t *testing.T) { } // Call ReplaceActions - got, updated, err := ReplaceActions(string(input)) + var got string + var updated bool + var replaceErr error + if tt.inputFile == "exemtedMaintainedActions.yml" { + got, updated, replaceErr = ReplaceActions(string(input), []string{"step-security/git-restore-mtime-action"}) + } else { + got, updated, replaceErr = ReplaceActions(string(input), []string{}) + } // Check error - if (err != nil) != tt.wantErr { - t.Errorf("ReplaceActions() error = %v, wantErr %v", err, tt.wantErr) + if (replaceErr != nil) != tt.wantErr { + t.Errorf("ReplaceActions() error = %v, wantErr %v", replaceErr, tt.wantErr) return } @@ -121,7 +114,7 @@ func TestReplaceActions(t *testing.T) { // Compare output with expected if got != string(expectedOutput) { - WriteYAML(tt.outputFile+"second", got) + // WriteYAML(tt.outputFile+"second", got) t.Errorf("ReplaceActions() = %v, want %v", got, string(expectedOutput)) } }) diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 1015269e2..052a60838 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -18,7 +18,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d pinActions, addHardenRunner, addPermissions, addProjectComment, addMaintainedActions := true, true, true, true, true pinnedActions, addedHardenRunner, addedPermissions, addedMaintainedActions := false, false, false, false ignoreMissingKBs := false - exemptedActions, pinToImmutable := []string{}, false + exemptedActions, pinToImmutable, customerMaintainedActions := []string{}, false, []string{} if len(params) > 0 { if v, ok := params[0].([]string); ok { exemptedActions = v @@ -29,6 +29,11 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d pinToImmutable = v } } + if len(params) > 2 { + if v, ok := params[2].([]string); ok { + customerMaintainedActions = v + } + } if queryStringParams["pinActions"] == "false" { pinActions = false @@ -79,7 +84,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d } if addMaintainedActions { - secureWorkflowReponse.FinalOutput, addedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput) + secureWorkflowReponse.FinalOutput, addedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput, customerMaintainedActions) if err != nil { secureWorkflowReponse.HasErrors = true } diff --git a/testfiles/maintainedActions/input/exemtedMaintainedActions.yml b/testfiles/maintainedActions/input/exemtedMaintainedActions.yml new file mode 100644 index 000000000..40dedd4b7 --- /dev/null +++ b/testfiles/maintainedActions/input/exemtedMaintainedActions.yml @@ -0,0 +1,35 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - 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: '**/*' + + build: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: '16' + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- + - uses: amannn/action-semantic-pull-request@v5 + with: + types: feat,fix,chore \ No newline at end of file diff --git a/testfiles/maintainedActions/output/exemtedMaintainedActions.yml b/testfiles/maintainedActions/output/exemtedMaintainedActions.yml new file mode 100644 index 000000000..f652f3945 --- /dev/null +++ b/testfiles/maintainedActions/output/exemtedMaintainedActions.yml @@ -0,0 +1,35 @@ +name: Test Workflow +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - 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: chetan/git-restore-mtime-action@v1 + with: + pattern: '**/*' + + build: + runs-on: ubuntu-latest + needs: test + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: '16' + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }} + restore-keys: | + ${{ runner.os }}-node- + - uses: step-security/action-semantic-pull-request@v5 + with: + types: feat,fix,chore \ No newline at end of file From e9bfbb3cbe0f712f1838f06f60c651601a4ad49d Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Mon, 12 May 2025 22:32:35 +0530 Subject: [PATCH 5/6] add actionMap as a argument to secure workflow --- .../maintainedactions/maintainedActions.go | 28 ++++--------------- .../maintainedactions_test.go | 20 ++++--------- remediation/workflow/secureworkflow.go | 18 +++++++----- remediation/workflow/secureworkflow_test.go | 19 +++++++++++-- 4 files changed, 39 insertions(+), 46 deletions(-) diff --git a/remediation/workflow/maintainedactions/maintainedActions.go b/remediation/workflow/maintainedactions/maintainedActions.go index feb867657..1c0d0ff86 100644 --- a/remediation/workflow/maintainedactions/maintainedActions.go +++ b/remediation/workflow/maintainedactions/maintainedActions.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "path/filepath" "strings" "github.com/step-security/secure-repo/remediation/workflow/metadata" @@ -32,10 +31,8 @@ type replacement struct { } // LoadMaintainedActions loads the maintained actions from the JSON file -func LoadMaintainedActions() (map[string]string, error) { +func LoadMaintainedActions(jsonPath string) (map[string]string, error) { // Read the JSON file - jsonPath := filepath.Join("maintainedactions", "maintainedActions.json") - // jsonPath := filepath.Join("maintainedActions.json") data, err := ioutil.ReadFile(jsonPath) if err != nil { return nil, fmt.Errorf("failed to read maintained actions file: %v", err) @@ -59,14 +56,13 @@ func LoadMaintainedActions() (map[string]string, error) { } // ReplaceActions replaces original actions with Step Security actions in a workflow -func ReplaceActions(inputYaml string, customerMaintainedActions []string) (string, bool, error) { +func ReplaceActions(inputYaml string, customerMaintainedActions map[string]string) (string, bool, error) { workflow := metadata.Workflow{} updated := false - actionMap, err := LoadMaintainedActions() - if err != nil { - return "", updated, fmt.Errorf("unable to load maintained actions: %v", err) - } - err = yaml.Unmarshal([]byte(inputYaml), &workflow) + + actionMap := customerMaintainedActions + + err := yaml.Unmarshal([]byte(inputYaml), &workflow) if err != nil { return "", updated, fmt.Errorf("unable to parse yaml: %v", err) } @@ -83,9 +79,6 @@ func ReplaceActions(inputYaml string, customerMaintainedActions []string) (strin // fmt.Println("step ", step.Uses) actionName := strings.Split(step.Uses, "@")[0] if newAction, ok := actionMap[actionName]; ok { - if isMaintained(newAction, customerMaintainedActions) { - continue - } latestVersion, err := GetLatestRelease(newAction) if err != nil { return "", updated, fmt.Errorf("unable to get latest release: %v", err) @@ -148,12 +141,3 @@ func replaceAction(t *yaml.Node, inputLines []string, replacements []replacement } return inputLines, updated } - -func isMaintained(actionName string, maintainedActions []string) bool { - for _, maintainedAction := range maintainedActions { - if maintainedAction == actionName { - return true - } - } - return false -} diff --git a/remediation/workflow/maintainedactions/maintainedactions_test.go b/remediation/workflow/maintainedactions/maintainedactions_test.go index 2e2a43281..522018592 100644 --- a/remediation/workflow/maintainedactions/maintainedactions_test.go +++ b/remediation/workflow/maintainedactions/maintainedactions_test.go @@ -69,12 +69,6 @@ func TestReplaceActions(t *testing.T) { wantUpdated: true, wantErr: false, }, - { - name: "exemtedMaintainedActions.yml", - inputFile: "exemtedMaintainedActions.yml", - outputFile: "exemtedMaintainedActions.yml", - wantUpdated: true, - }, } for _, tt := range tests { @@ -84,16 +78,12 @@ func TestReplaceActions(t *testing.T) { if err != nil { t.Fatalf("error reading input file: %v", err) } - - // Call ReplaceActions - var got string - var updated bool - var replaceErr error - if tt.inputFile == "exemtedMaintainedActions.yml" { - got, updated, replaceErr = ReplaceActions(string(input), []string{"step-security/git-restore-mtime-action"}) - } else { - got, updated, replaceErr = ReplaceActions(string(input), []string{}) + actionMap, err := LoadMaintainedActions("maintainedActions.json") + if err != nil { + t.Errorf("ReplaceActions() unable to json file %v", err) + return } + got, updated, replaceErr := ReplaceActions(string(input), actionMap) // Check error if (replaceErr != nil) != tt.wantErr { diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 052a60838..4a063b25c 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -15,10 +15,10 @@ const ( ) func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc dynamodbiface.DynamoDBAPI, params ...interface{}) (*permissions.SecureWorkflowReponse, error) { - pinActions, addHardenRunner, addPermissions, addProjectComment, addMaintainedActions := true, true, true, true, true - pinnedActions, addedHardenRunner, addedPermissions, addedMaintainedActions := false, false, false, false + pinActions, addHardenRunner, addPermissions, addProjectComment, replaceMaintainedActions := true, true, true, true, false + pinnedActions, addedHardenRunner, addedPermissions, replacedMaintainedActions := false, false, false, false ignoreMissingKBs := false - exemptedActions, pinToImmutable, customerMaintainedActions := []string{}, false, []string{} + exemptedActions, pinToImmutable, customerMaintainedActions := []string{}, false, map[string]string{} if len(params) > 0 { if v, ok := params[0].([]string); ok { exemptedActions = v @@ -30,7 +30,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d } } if len(params) > 2 { - if v, ok := params[2].([]string); ok { + if v, ok := params[2].(map[string]string); ok { customerMaintainedActions = v } } @@ -55,6 +55,10 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d addProjectComment = false } + if len(customerMaintainedActions) > 0 { + replaceMaintainedActions = true + } + secureWorkflowReponse := &permissions.SecureWorkflowReponse{FinalOutput: inputYaml, OriginalInput: inputYaml} var err error if addPermissions { @@ -83,8 +87,8 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d addedPermissions = !secureWorkflowReponse.HasErrors } - if addMaintainedActions { - secureWorkflowReponse.FinalOutput, addedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput, customerMaintainedActions) + if replaceMaintainedActions { + secureWorkflowReponse.FinalOutput, replacedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput, customerMaintainedActions) if err != nil { secureWorkflowReponse.HasErrors = true } @@ -110,6 +114,6 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d secureWorkflowReponse.PinnedActions = pinnedActions secureWorkflowReponse.AddedHardenRunner = addedHardenRunner secureWorkflowReponse.AddedPermissions = addedPermissions - secureWorkflowReponse.AddedMaintainedActions = addedMaintainedActions + secureWorkflowReponse.AddedMaintainedActions = replacedMaintainedActions return secureWorkflowReponse, nil } diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index aab090631..6be2e663c 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/jarcoal/httpmock" + "github.com/step-security/secure-repo/remediation/workflow/maintainedactions" + "github.com/step-security/secure-repo/remediation/workflow/permissions" ) func TestSecureWorkflow(t *testing.T) { @@ -202,7 +204,9 @@ func TestSecureWorkflow(t *testing.T) { {fileName: "error.yml", wantPinnedActions: false, wantAddedHardenRunner: false, wantAddedPermissions: false}, } for _, test := range tests { - input, err := ioutil.ReadFile(path.Join(inputDirectory, test.fileName)) + var err error + var input []byte + input, err = ioutil.ReadFile(path.Join(inputDirectory, test.fileName)) if err != nil { log.Fatal(err) @@ -232,7 +236,18 @@ func TestSecureWorkflow(t *testing.T) { } queryParams["addProjectComment"] = "false" - output, err := SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}) + var output *permissions.SecureWorkflowReponse + var actionMap map[string]string + if test.fileName == "oneJob.yml" { + actionMap, err = maintainedactions.LoadMaintainedActions("maintainedactions/maintainedActions.json") + if err != nil { + t.Errorf("unable to load the file %s", err) + } + output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}, []string{}, false, actionMap) + + } else { + output, err = SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{}) + } if err != nil { t.Errorf("Error not expected") From b1012add8ff338c2478ce47ea95961754a4a94d7 Mon Sep 17 00:00:00 2001 From: Balijepalli Vamshi Krishna Date: Tue, 13 May 2025 22:46:20 +0530 Subject: [PATCH 6/6] add name changes to variables --- remediation/workflow/secureworkflow.go | 13 +++++++------ remediation/workflow/secureworkflow_test.go | 6 +++--- .../input/{oneJob.yml => replaceactions.yml} | 0 .../output/{oneJob.yml => replaceactions.yml} | 0 4 files changed, 10 insertions(+), 9 deletions(-) rename testfiles/secureworkflow/input/{oneJob.yml => replaceactions.yml} (100%) rename testfiles/secureworkflow/output/{oneJob.yml => replaceactions.yml} (100%) diff --git a/remediation/workflow/secureworkflow.go b/remediation/workflow/secureworkflow.go index 621b683f6..114e34601 100644 --- a/remediation/workflow/secureworkflow.go +++ b/remediation/workflow/secureworkflow.go @@ -21,8 +21,8 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d pinActions, addHardenRunner, addPermissions, addProjectComment, replaceMaintainedActions := true, true, true, true, false pinnedActions, addedHardenRunner, addedPermissions, replacedMaintainedActions := false, false, false, false ignoreMissingKBs := false - enableLogging := false - exemptedActions, pinToImmutable, customerMaintainedActions := []string{}, false, map[string]string{} + enableLogging := false + exemptedActions, pinToImmutable, maintainedActionsMap := []string{}, false, map[string]string{} if len(params) > 0 { if v, ok := params[0].([]string); ok { @@ -36,7 +36,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d } if len(params) > 2 { if v, ok := params[2].(map[string]string); ok { - customerMaintainedActions = v + maintainedActionsMap = v } } @@ -60,9 +60,10 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d addProjectComment = false } - if len(customerMaintainedActions) > 0 { + if len(maintainedActionsMap) > 0 { replaceMaintainedActions = true - + } + if queryStringParams["enableLogging"] == "true" { enableLogging = true } @@ -120,7 +121,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d } if replaceMaintainedActions { - secureWorkflowReponse.FinalOutput, replacedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput, customerMaintainedActions) + secureWorkflowReponse.FinalOutput, replacedMaintainedActions, err = maintainedactions.ReplaceActions(secureWorkflowReponse.FinalOutput, maintainedActionsMap) if err != nil { secureWorkflowReponse.HasErrors = true } diff --git a/remediation/workflow/secureworkflow_test.go b/remediation/workflow/secureworkflow_test.go index 6be2e663c..7a739889d 100644 --- a/remediation/workflow/secureworkflow_test.go +++ b/remediation/workflow/secureworkflow_test.go @@ -193,7 +193,7 @@ func TestSecureWorkflow(t *testing.T) { wantAddedPermissions bool wantAddedMaintainedActions bool }{ - {fileName: "oneJob.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true}, + {fileName: "replaceactions.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false, wantAddedMaintainedActions: true}, {fileName: "allscenarios.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: true}, {fileName: "missingaction.yml", wantPinnedActions: true, wantAddedHardenRunner: true, wantAddedPermissions: false}, {fileName: "nohardenrunner.yml", wantPinnedActions: true, wantAddedHardenRunner: false, wantAddedPermissions: true}, @@ -228,7 +228,7 @@ func TestSecureWorkflow(t *testing.T) { case "multiplejobperms.yml": queryParams["addHardenRunner"] = "false" queryParams["pinActions"] = "false" - case "oneJob.yml": + case "replaceactions.yml": queryParams["addMaintainedActions"] = "true" queryParams["addHardenRunner"] = "true" queryParams["pinActions"] = "true" @@ -238,7 +238,7 @@ func TestSecureWorkflow(t *testing.T) { var output *permissions.SecureWorkflowReponse var actionMap map[string]string - if test.fileName == "oneJob.yml" { + if test.fileName == "replaceactions.yml" { actionMap, err = maintainedactions.LoadMaintainedActions("maintainedactions/maintainedActions.json") if err != nil { t.Errorf("unable to load the file %s", err) diff --git a/testfiles/secureworkflow/input/oneJob.yml b/testfiles/secureworkflow/input/replaceactions.yml similarity index 100% rename from testfiles/secureworkflow/input/oneJob.yml rename to testfiles/secureworkflow/input/replaceactions.yml diff --git a/testfiles/secureworkflow/output/oneJob.yml b/testfiles/secureworkflow/output/replaceactions.yml similarity index 100% rename from testfiles/secureworkflow/output/oneJob.yml rename to testfiles/secureworkflow/output/replaceactions.yml