From cf30f20397a28ae51503b1a2627b95b03eb9c1a8 Mon Sep 17 00:00:00 2001 From: Pedro Kaj Kjellerup Nacht Date: Sun, 10 Nov 2024 16:57:06 -0500 Subject: [PATCH] :sparkles: Add machine-readable patch to fix script injections in workflows (#4218) * Merge pull request #1 from joycebrum/feature/setup-environment-for-dw-fix create environment for patch on DW script injections Signed-off-by: Diogo Teles Sant'Anna Signed-off-by: Pedro Kaj Kjellerup Nacht * Merge pull request #3 from joycebrum/feat/connect-patch-generator-with-remediation-output Include the generated patch in the output Signed-off-by: Joyce Brum Signed-off-by: Pedro Kaj Kjellerup Nacht * Merge pull request #2 from joycebrum/test/initial-tests-for-dw-fix Signed-off-by: Pedro Kaj Kjellerup Nacht * Merge pull request #4 from joycebrum/feat/get-input-needed-to-generate-patch Signed-off-by: Pedro Kaj Kjellerup Nacht * impl.go: slight refactor to loop Signed-off-by: Pedro Kaj Kjellerup Nacht * Add envvars to existing or new env, still not replaced in `run` Signed-off-by: Pedro Kaj Kjellerup Nacht * Replace unsafe variables in run commands, generate git diff Git diff created using hexops/gotextdiff, WHICH IS ARCHIVED. It is unfortunately the only package I found which could do it. To be discussed with Scorecard maintainers whether it's worth it. Signed-off-by: Pedro Kaj Kjellerup Nacht * Rewrite test file - Test patchWorkflow instead of GeneratePatch. This avoids the complication of comparing diff files; we can instead simply compare the output workflow to an expected "fixed" workflow. - Examples with multiple findings must have separate "fixed" workflows for each finding, not a single file which covers all findings - Instead of hard-coding the finding details (snippet, line position), run raw.DangerousWorkflow() to get that data automatically. This does make these tests a bit more "integration-test-like", but makes them substantially easier to maintain. Signed-off-by: Pedro Kaj Kjellerup Nacht * Rewrite patch/impl.go - misc refactors - use go-git to generate diff - Most functions now return errors instead of bools. This can be later used for simpler logging - Existing environment variables are now detected by parsing the files as GH workflows. This is WIP to handle existing envvars in our patches. - Remove instances of C-style for-loops, unnecessarily dangerous! - Fixed proper detection of existing env, handling blank lines and comments. Signed-off-by: Pedro Kaj Kjellerup Nacht * Update test workflows - Fix inconsistencies between original and "fixed" versions - Store multiple "fixed" workflows for tests with multiple findings. Each "fixed" workflow fixes a single finding. The files are numbered according to the order in which the findings are found by moving down the file. - allKindsOfUserInput removed. Would require too many "fixed" workflows to test. The behavior can be tested more directly. Signed-off-by: Pedro Kaj Kjellerup Nacht * Use existing envvars, validate patched workflow - If an envvar with our name and value already existed but simply wasn't used, the patch no longer duplicates it. - After the patched workflow is created, we validate that it is valid. Or, at least did not introduce any syntax errors that were not present in the original workflow. Signed-off-by: Pedro Kaj Kjellerup Nacht * Test for same injection in same step, leading to duplicate findings Signed-off-by: Pedro Kaj Kjellerup Nacht * Use existing envvars with different name but same meaning Signed-off-by: Pedro Kaj Kjellerup Nacht * Avoid conflicts with irrelevant but existing envvars Signed-off-by: Pedro Kaj Kjellerup Nacht * Use first job's indent to define envvar indent Signed-off-by: Pedro Kaj Kjellerup Nacht * Refactor patch/impl_test - Create helper function `readWorkflow` - Improved error handling in case of failed workflow validation - Allow the declaration of duplicate findings (cases where 2+ findings have the same patch) Signed-off-by: Pedro Kaj Kjellerup Nacht * patch/impl: Simplify unsafePatterns, use errors, docs, lint - Simplify use of unsafePatterns - Replaced boolean returns with errors, for easier log/debugging - Improved documentation - Changes to satisfy linter, adoption of 120-char line limit Signed-off-by: Pedro Kaj Kjellerup Nacht * Fix panic in hasScriptInjection test due to missing file Signed-off-by: Pedro Kaj Kjellerup Nacht * Avoid duplicate envvars dealing with array variables Signed-off-by: Pedro Kaj Kjellerup Nacht * Adopt existing inter-block spacing for new env Signed-off-by: Pedro Kaj Kjellerup Nacht * chore: Tidy up function order, remove unused files Signed-off-by: Pedro Kaj Kjellerup Nacht * Define localPath in runScorecard Signed-off-by: Pedro Kaj Kjellerup Nacht * Assert valid offset, use TrimSpace, drop unused struct member Signed-off-by: Pedro Kaj Kjellerup Nacht * Just use []bytes instead of string Signed-off-by: Pedro Kaj Kjellerup Nacht * Use []byte, not string Signed-off-by: Pedro Kaj Kjellerup Nacht * go mod tidy updates Signed-off-by: Pedro Kaj Kjellerup Nacht * Ensure valid offset Signed-off-by: Pedro Kaj Kjellerup Nacht * Move /patch to /internal/patch Signed-off-by: Pedro Kaj Kjellerup Nacht * Document patch behavior and add patch to remediation in def.yml Signed-off-by: Pedro Kaj Kjellerup Nacht * Updates from review Signed-off-by: Pedro Kaj Kjellerup Nacht * Add patch to finding before adding to list of findings Signed-off-by: Pedro Kaj Kjellerup Nacht --------- Signed-off-by: Diogo Teles Sant'Anna Signed-off-by: Pedro Kaj Kjellerup Nacht Signed-off-by: Joyce Brum Co-authored-by: Diogo Teles Sant'Anna Co-authored-by: Joyce Brum --- cmd/internal/scdiff/app/runner/runner_test.go | 1 + docs/probes.md | 2 +- go.mod | 2 +- pkg/scorecard/scorecard.go | 6 + pkg/scorecard/scorecard_test.go | 4 + .../def.yml | 7 +- .../impl.go | 95 ++- .../impl_test.go | 3 + .../internal/patch/impl.go | 584 ++++++++++++++++++ .../internal/patch/impl_test.go | 241 ++++++++ .../patch/testdata/arrayVariables.yaml | 27 + .../testdata/arrayVariables_fixed_1.yaml | 30 + .../testdata/arrayVariables_fixed_2.yaml | 30 + .../testdata/crazyButValidIndentation.yaml | 37 ++ .../crazyButValidIndentation_fixed.yaml | 41 ++ .../testdata/envVarNameAlreadyInUse.yaml | 35 ++ .../envVarNameAlreadyInUse_fixed.yaml | 36 ++ .../fourSpacesIndentationExistentEnvVar.yaml | 27 + ...SpacesIndentationExistentEnvVar_fixed.yaml | 28 + .../testdata/ignorePatternInsideComments.yaml | 24 + .../internal/patch/testdata/newlineOnEOF.yaml | 26 + .../patch/testdata/newlineOnEOF_fixed.yaml | 29 + .../testdata/noLineBreaksBetweenBlocks.yaml | 24 + .../noLineBreaksBetweenBlocks_fixed.yaml | 26 + .../internal/patch/testdata/realExample1.yaml | 58 ++ .../patch/testdata/realExample1_fixed.yaml | 61 ++ .../internal/patch/testdata/realExample2.yaml | 99 +++ .../patch/testdata/realExample2_fixed.yaml | 102 +++ .../internal/patch/testdata/realExample3.yaml | 44 ++ .../patch/testdata/realExample3_fixed.yaml | 47 ++ .../testdata/reuseEnvVarSmallerScope.yaml | 53 ++ .../reuseEnvVarSmallerScope_fixed.yaml | 53 ++ .../testdata/reuseEnvVarWithDiffName.yaml | 31 + .../reuseEnvVarWithDiffName_fixed.yaml | 31 + .../testdata/reuseWorkflowLevelEnvVars.yaml | 56 ++ .../reuseWorkflowLevelEnvVars_fixed_1.yaml | 56 ++ .../reuseWorkflowLevelEnvVars_fixed_2.yaml | 57 ++ .../reuseWorkflowLevelEnvVars_fixed_3.yaml | 57 ++ .../testdata/twoInjectionsDifferentJobs.yaml | 28 + .../twoInjectionsDifferentJobs_fixed_1.yaml | 31 + .../twoInjectionsDifferentJobs_fixed_2.yaml | 31 + .../patch/testdata/twoInjectionsSameJob.yaml | 26 + .../twoInjectionsSameJob_fixed_1.yaml | 29 + .../twoInjectionsSameJob_fixed_2.yaml | 29 + .../patch/testdata/twoInjectionsSameStep.yaml | 26 + .../twoInjectionsSameStep_fixed_1.yaml | 29 + .../twoInjectionsSameStep_fixed_3.yaml | 29 + .../testdata/userInputAssignedToVariable.yaml | 31 + .../userInputAssignedToVariable_fixed.yaml | 34 + 49 files changed, 2474 insertions(+), 19 deletions(-) create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/impl_test.go create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_2.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/ignorePatternInsideComments.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName_fixed.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_2.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_3.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_2.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_2.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_1.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_3.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable.yaml create mode 100644 probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable_fixed.yaml diff --git a/cmd/internal/scdiff/app/runner/runner_test.go b/cmd/internal/scdiff/app/runner/runner_test.go index 1a43c8e3f19..f8123f9af58 100644 --- a/cmd/internal/scdiff/app/runner/runner_test.go +++ b/cmd/internal/scdiff/app/runner/runner_test.go @@ -46,6 +46,7 @@ func TestRunner_Run(t *testing.T) { mockRepo.EXPECT().GetDefaultBranchName().Return("main", nil) mockRepo.EXPECT().Close().Return(nil) mockRepo.EXPECT().GetFileReader(gomock.Any()).Return(nil, errors.New("reading files unsupported for this test")).AnyTimes() + mockRepo.EXPECT().LocalPath().Return(".", nil) r := Runner{ ctx: context.Background(), // use a check which works locally, but we declare no files above so no-op diff --git a/docs/probes.md b/docs/probes.md index 7bb01e85646..5bdbaf61ad4 100644 --- a/docs/probes.md +++ b/docs/probes.md @@ -194,7 +194,7 @@ If the probe finds no binary files, it returns a single OutcomeFalse. **Implementation**: The probe analyzes the repository's workflows for known dangerous patterns. -**Outcomes**: The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. +**Outcomes**: The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. Each finding may include a suggested patch to fix the respective script injection. If no dangerous patterns are found, the probe returns one finding with OutcomeFalse. diff --git a/go.mod b/go.mod index 5503beb21e7..2946c6dfcc6 100644 --- a/go.mod +++ b/go.mod @@ -150,7 +150,7 @@ require ( github.com/emirpasic/gods v1.18.1 // indirect github.com/fatih/color v1.17.0 // indirect github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect - github.com/go-git/go-billy/v5 v5.5.0 // indirect + github.com/go-git/go-billy/v5 v5.5.0 github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v4 v4.5.1 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect diff --git a/pkg/scorecard/scorecard.go b/pkg/scorecard/scorecard.go index 8cb113fcf34..d3a73291d4e 100644 --- a/pkg/scorecard/scorecard.go +++ b/pkg/scorecard/scorecard.go @@ -138,6 +138,11 @@ func runScorecard(ctx context.Context, resultsCh := make(chan checker.CheckResult) + localPath, err := repoClient.LocalPath() + if err != nil { + return Result{}, fmt.Errorf("RepoClient.LocalPath: %w", err) + } + // Set metadata for all checks to use. This is necessary // to create remediations from the probe yaml files. ret.RawResults.Metadata.Metadata = map[string]string{ @@ -146,6 +151,7 @@ func runScorecard(ctx context.Context, "repository.uri": repo.URI(), "repository.sha1": commitSHA, "repository.defaultBranch": defaultBranch, + "localPath": localPath, } request := &checker.CheckRequest{ diff --git a/pkg/scorecard/scorecard_test.go b/pkg/scorecard/scorecard_test.go index 2ea5a973b1c..3435bb54966 100644 --- a/pkg/scorecard/scorecard_test.go +++ b/pkg/scorecard/scorecard_test.go @@ -242,6 +242,7 @@ func TestRun_WithProbes(t *testing.T) { "repository.name": "ossf/scorecard", "repository.sha1": "1a17bb812fb2ac23e9d09e86e122f8b67563aed7", "repository.uri": "github.com/ossf/scorecard", + "localPath": "test_path", }, }, }, @@ -279,6 +280,9 @@ func TestRun_WithProbes(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().LocalPath().DoAndReturn(func() (string, error) { + return "test_path", nil + }).AnyTimes() repo := mockrepo.NewMockRepo(ctrl) repo.EXPECT().URI().Return(tt.args.uri).AnyTimes() diff --git a/probes/hasDangerousWorkflowScriptInjection/def.yml b/probes/hasDangerousWorkflowScriptInjection/def.yml index 7c4f482ae0c..3b0b0351702 100644 --- a/probes/hasDangerousWorkflowScriptInjection/def.yml +++ b/probes/hasDangerousWorkflowScriptInjection/def.yml @@ -20,7 +20,7 @@ motivation: > implementation: > The probe analyzes the repository's workflows for known dangerous patterns. outcome: - - The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. + - The probe returns one finding with OutcomeTrue for each dangerous script injection pattern detected. Each finding may include a suggested patch to fix the respective script injection. - If no dangerous patterns are found, the probe returns one finding with OutcomeFalse. remediation: onOutcome: True @@ -30,6 +30,11 @@ remediation: markdown: - Avoid the dangerous workflow patterns. - See [this document](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections) for information on avoiding and mitigating the risk of script injections. + - | + Here is a proposed patch to eliminate this risk: + ```yml + ${{ metadata.patch }} + ``` ecosystem: languages: - all diff --git a/probes/hasDangerousWorkflowScriptInjection/impl.go b/probes/hasDangerousWorkflowScriptInjection/impl.go index 1ec39da97dc..f0396849582 100644 --- a/probes/hasDangerousWorkflowScriptInjection/impl.go +++ b/probes/hasDangerousWorkflowScriptInjection/impl.go @@ -18,11 +18,16 @@ package hasDangerousWorkflowScriptInjection import ( "embed" "fmt" + "os" + "path" + + "github.com/rhysd/actionlint" "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/finding" "github.com/ossf/scorecard/v5/internal/checknames" "github.com/ossf/scorecard/v5/internal/probes" + "github.com/ossf/scorecard/v5/probes/hasDangerousWorkflowScriptInjection/internal/patch" "github.com/ossf/scorecard/v5/probes/internal/utils/uerror" ) @@ -53,23 +58,37 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { } var findings []finding.Finding - for _, e := range r.Workflows { - e := e - if e.Type == checker.DangerousWorkflowScriptInjection { - f, err := finding.NewWith(fs, Probe, - fmt.Sprintf("script injection with untrusted input '%v'", e.File.Snippet), - nil, finding.OutcomeTrue) - if err != nil { - return nil, Probe, fmt.Errorf("create finding: %w", err) - } - f = f.WithLocation(&finding.Location{ - Path: e.File.Path, - Type: e.File.Type, - LineStart: &e.File.Offset, - Snippet: &e.File.Snippet, - }) - findings = append(findings, *f) + var currWorkflow string + var workflow *actionlint.Workflow + var content []byte + var errs []*actionlint.Error + localPath := raw.Metadata.Metadata["localPath"] + for _, w := range r.Workflows { + w := w + if w.Type != checker.DangerousWorkflowScriptInjection { + continue + } + + f, err := finding.NewWith(fs, Probe, + fmt.Sprintf("script injection with untrusted input '%v'", w.File.Snippet), + nil, finding.OutcomeTrue) + if err != nil { + return nil, Probe, fmt.Errorf("create finding: %w", err) } + + f = f.WithLocation(&finding.Location{ + Path: w.File.Path, + Type: w.File.Type, + LineStart: &w.File.Offset, + Snippet: &w.File.Snippet, + }) + + err = parseWorkflow(localPath, &w, &currWorkflow, &content, &workflow, &errs) + if err == nil { + generatePatch(&w, content, workflow, errs, f) + } + + findings = append(findings, *f) } if len(findings) == 0 { @@ -79,6 +98,50 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { return findings, Probe, nil } +func parseWorkflow( + localPath string, + e *checker.DangerousWorkflow, + currWorkflow *string, + content *[]byte, + workflow **actionlint.Workflow, + errs *[]*actionlint.Error, +) error { + var err error + wp := path.Join(localPath, e.File.Path) + if *currWorkflow != wp { + // update current open file if injection in different file + *currWorkflow = wp + *content, err = os.ReadFile(wp) + if err != nil { + return err //nolint:wrapcheck // we only care about the error's existence + } + + *workflow, *errs = actionlint.Parse(*content) + if len(*errs) > 0 && *workflow == nil { + // the workflow contains unrecoverable parsing errors, skip. + return err //nolint:wrapcheck // we only care about the error's existence + } + } + return nil +} + +func generatePatch( + e *checker.DangerousWorkflow, + content []byte, + workflow *actionlint.Workflow, + errs []*actionlint.Error, + f *finding.Finding, +) { + findingPatch, err := patch.GeneratePatch(e.File, content, workflow, errs) + if err != nil { + return + } + f.WithPatch(&findingPatch) + f.WithRemediationMetadata(map[string]string{ + "patch": findingPatch, + }) +} + func falseOutcome() ([]finding.Finding, string, error) { f, err := finding.NewWith(fs, Probe, "Project does not have dangerous workflow(s) with possibility of script injection.", nil, diff --git a/probes/hasDangerousWorkflowScriptInjection/impl_test.go b/probes/hasDangerousWorkflowScriptInjection/impl_test.go index 06a8651b5ff..d89adc8a90c 100644 --- a/probes/hasDangerousWorkflowScriptInjection/impl_test.go +++ b/probes/hasDangerousWorkflowScriptInjection/impl_test.go @@ -43,6 +43,9 @@ func Test_Run(t *testing.T) { Workflows: []checker.DangerousWorkflow{ { Type: checker.DangerousWorkflowScriptInjection, + File: checker.File{ + Path: "patch/testdata/userInputAssignedToVariable.yaml", + }, }, }, }, diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go new file mode 100644 index 00000000000..5f464039616 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl.go @@ -0,0 +1,584 @@ +// Copyright 2024 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package patch + +import ( + "bytes" + "fmt" + "regexp" + "slices" + "strings" + + "github.com/go-git/go-billy/v5/memfs" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/storage/memory" + "github.com/rhysd/actionlint" + + "github.com/ossf/scorecard/v5/checker" + "github.com/ossf/scorecard/v5/checks/fileparser" + sce "github.com/ossf/scorecard/v5/errors" +) + +type unsafePattern struct { + idRegex *regexp.Regexp + replaceRegex *regexp.Regexp + envvarName string +} + +// Fixes the script injection identified by the finding and returns a unified diff users can apply (with `git apply` or +// `patch`) to fix the workflow themselves. Should an error occur, an empty patch is returned. +func GeneratePatch( + f checker.File, + content []byte, + workflow *actionlint.Workflow, + workflowErrs []*actionlint.Error, +) (string, error) { + patchedWorkflow, err := patchWorkflow(f, content, workflow) + if err != nil { + return "", err + } + errs := validatePatchedWorkflow(patchedWorkflow, workflowErrs) + if len(errs) > 0 { + return "", fileparser.FormatActionlintError(errs) + } + return getDiff(f.Path, content, patchedWorkflow) +} + +// Returns a patched version of the workflow without the script injection finding. +func patchWorkflow(f checker.File, content []byte, workflow *actionlint.Workflow) ([]byte, error) { + unsafeVar := strings.TrimSpace(f.Snippet) + + lines := bytes.Split(content, []byte("\n")) + runCmdIndex := int(f.Offset - 1) + + if runCmdIndex < 0 || runCmdIndex >= len(lines) { + return []byte(""), sce.WithMessage(sce.ErrScorecardInternal, "Invalid dangerous workflow offset") + } + + unsafePattern, err := getUnsafePattern(unsafeVar) + if err != nil { + return []byte(""), err + } + + existingEnvvars := parseExistingEnvvars(workflow) + unsafePattern, err = useExistingEnvvars(unsafePattern, existingEnvvars, unsafeVar) + if err != nil { + return []byte(""), err + } + + replaceUnsafeVarWithEnvvar(lines, unsafePattern, runCmdIndex) + + lines, err = addEnvvarToGlobalEnv(lines, existingEnvvars, unsafePattern, unsafeVar) + if err != nil { + return []byte(""), sce.WithMessage(sce.ErrScorecardInternal, + fmt.Sprintf("Unknown dangerous variable: %s", unsafeVar)) + } + + return bytes.Join(lines, []byte("\n")), nil +} + +func getUnsafePattern(unsafeVar string) (unsafePattern, error) { + unsafePatterns := []unsafePattern{ + newUnsafePattern("AUTHOR_EMAIL", `github\.event\.commits.*?\.author\.email`), + newUnsafePattern("AUTHOR_EMAIL", `github\.event\.head_commit\.author\.email`), + newUnsafePattern("AUTHOR_NAME", `github\.event\.commits.*?\.author\.name`), + newUnsafePattern("AUTHOR_NAME", `github\.event\.head_commit\.author\.name`), + newUnsafePattern("COMMENT_BODY", `github\.event\.comment\.body`), + newUnsafePattern("COMMIT_MESSAGE", `github\.event\.commits.*?\.message`), + newUnsafePattern("COMMIT_MESSAGE", `github\.event\.head_commit\.message`), + newUnsafePattern("DISCUSSION_TITLE", `github\.event\.discussion\.title`), + newUnsafePattern("DISCUSSION_BODY", `github\.event\.discussion\.body`), + newUnsafePattern("ISSUE_BODY", `github\.event\.issue\.body`), + newUnsafePattern("ISSUE_COMMENT_BODY", `github\.event\.issue_comment\.comment\.body`), + newUnsafePattern("ISSUE_TITLE", `github\.event\.issue\.title`), + newUnsafePattern("PAGE_NAME", `github\.event\.pages.*?\.page_name`), + newUnsafePattern("PR_BODY", `github\.event\.pull_request\.body`), + newUnsafePattern("PR_DEFAULT_BRANCH", `github\.event\.pull_request\.head\.repo\.default_branch`), + newUnsafePattern("PR_HEAD_LABEL", `github\.event\.pull_request\.head\.label`), + newUnsafePattern("PR_HEAD_REF", `github\.event\.pull_request\.head\.ref`), + newUnsafePattern("PR_TITLE", `github\.event\.pull_request\.title`), + newUnsafePattern("REVIEW_BODY", `github\.event\.review\.body`), + newUnsafePattern("REVIEW_COMMENT_BODY", `github\.event\.review_comment\.body`), + + newUnsafePattern("HEAD_REF", `github\.head_ref`), + } + + for _, p := range unsafePatterns { + p := p + if p.idRegex.MatchString(unsafeVar) { + arrayVarRegex := regexp.MustCompile(`\[(.+?)\]`) + arrayIdx := arrayVarRegex.FindStringSubmatch(unsafeVar) + if arrayIdx == nil || len(arrayIdx) < 2 { + // not an array variable, the default envvar name is sufficient. + return p, nil + } + // Array variable (i.e. `github.event.commits[0].message`), must avoid potential conflicts. + // Add the array index to the name as a suffix, and use the exact unsafe variable name instead of the + // default, which includes a regex that will catch all instances of the array. + envvarName := fmt.Sprintf("%s_%s", p.envvarName, arrayIdx[1]) + return newUnsafePattern(envvarName, regexp.QuoteMeta(unsafeVar)), nil + } + } + + return unsafePattern{}, sce.WithMessage(sce.ErrScorecardInternal, + fmt.Sprintf("Unknown dangerous variable: %s", unsafeVar)) +} + +func newUnsafePattern(e, p string) unsafePattern { + return unsafePattern{ + envvarName: e, + // Regex to simply identify the unsafe variable that triggered the finding. + // Must use a regex and not a simple string to identify possible uses of array variables + // (i.e. `github.event.commits[0].author.email`). + idRegex: regexp.MustCompile(p), + // Regex to replace the unsafe variable in a `run` command with the envvar name. + replaceRegex: regexp.MustCompile(`{{\s*.*?` + p + `.*?\s*}}`), + } +} + +// Parses the envvars from the existing global `env:` block. +// Returns a map from the GitHub variable name to the envvar name (i.e. "github.event.issue.body": "ISSUE_BODY"). +func parseExistingEnvvars(workflow *actionlint.Workflow) map[string]string { + envvars := make(map[string]string) + + if workflow.Env == nil { + return envvars + } + + r := regexp.MustCompile(`\$\{\{\s*(github\.[^\s]*?)\s*}}`) + for _, v := range workflow.Env.Vars { + value := v.Value.Value + + if strings.Contains(value, "${{") { + // extract simple variable definition (without brackets, etc) + m := r.FindStringSubmatch(value) + if len(m) == 2 { + value = m[1] + envvars[value] = v.Name.Value + } else { + envvars[v.Value.Value] = v.Name.Value + } + } else { + envvars[v.Value.Value] = v.Name.Value + } + } + + return envvars +} + +// Identifies whether the original workflow contains envvars which may conflict with our patch. +// Should an existing envvar already handle our dangerous variable, it will be used in the patch instead of creating a +// new envvar with the same value. +// Should an existing envvar have the same name as the one that would ordinarily be used by the patch, the patch appends +// a suffix to the patch's envvar name to avoid conflicts. +// +// Returns the unsafePattern, possibly updated to consider the existing envvars. +func useExistingEnvvars( + pattern unsafePattern, + existingEnvvars map[string]string, + unsafeVar string, +) (unsafePattern, error) { + if envvar, ok := existingEnvvars[unsafeVar]; ok { + // There already exists an envvar handling our unsafe variable. + // Use that envvar instead of creating another one with the same value. + pattern.envvarName = envvar + return pattern, nil + } + + // If there's an envvar with the same name as what we'd use, add a hard-coded suffix to our name to avoid conflicts. + // Clumsy but works in almost all cases, and should be rare. + for _, e := range existingEnvvars { + if e == pattern.envvarName { + pattern.envvarName += "_1" + return pattern, nil + } + } + + return pattern, nil +} + +// Replaces all instances of the given script injection variable with the safe environment variable. +func replaceUnsafeVarWithEnvvar(lines [][]byte, pattern unsafePattern, runIndex int) { + runIndent := getIndent(lines[runIndex]) + for i, line := range lines[runIndex:] { + currLine := runIndex + i + if i > 0 && isParentLevelIndent(lines[currLine], runIndent) { + // anything at the same indent as the first line of the `- run:` block will mean the end of the run block. + break + } + lines[currLine] = pattern.replaceRegex.ReplaceAll(line, []byte(pattern.envvarName)) + } +} + +// Adds the necessary environment variable to the global `env:` block. +// If the `env:` block does not exist, it is created right above the `jobs:` block. +// +// Returns the new array of lines describing the workflow after inserting the new envvar. +func addEnvvarToGlobalEnv( + lines [][]byte, + existingEnvvars map[string]string, + pattern unsafePattern, unsafeVar string, +) ([][]byte, error) { + globalIndentation, err := findGlobalIndentation(lines) + if err != nil { + return lines, err + } + + if _, ok := existingEnvvars[unsafeVar]; ok { + // an existing envvar already handles this unsafe var, we can simply use it + return lines, nil + } + + var insertPos, envvarIndent int + if len(existingEnvvars) > 0 { + insertPos, envvarIndent = findExistingEnv(lines, globalIndentation) + } else { + lines, insertPos, err = addNewGlobalEnv(lines, globalIndentation) + if err != nil { + return lines, err + } + + // position now points to `env:`, insert variables below it + insertPos++ + envvarIndent = globalIndentation + getDefaultIndentStep(lines) + } + + envvarDefinition := fmt.Sprintf("%s: ${{ %s }}", pattern.envvarName, unsafeVar) + lines = slices.Insert(lines, insertPos, append(bytes.Repeat([]byte(" "), envvarIndent), []byte(envvarDefinition)...)) + + return lines, nil +} + +// Detects where the existing global `env:` block is located. +// +// Returns: +// - int: the index for the line where a new global envvar should be added (after the last existing envvar) +// - int: the indentation used for the declared environment variables +// +// Both values return -1 if the `env` block doesn't exist or is invalid. +func findExistingEnv(lines [][]byte, globalIndent int) (int, int) { + var currPos int + var line []byte + envRegex := labelRegex("env", globalIndent) + for currPos, line = range lines { + if envRegex.Match(line) { + break + } + } + + if currPos >= len(lines)-1 { + // Invalid env, there must be at least one more line for an existing envvar. Shouldn't happen. + return -1, -1 + } + + currPos++ // move to line after `env:` + insertPos := currPos // marks the position where new envvars will be added + var envvarIndent int + for i, line := range lines[currPos:] { + if isBlankOrComment(line) { + continue + } + + if isParentLevelIndent(line, globalIndent) { + // no longer declaring envvars + break + } + + envvarIndent = getIndent(line) + insertPos = currPos + i + 1 + } + + return insertPos, envvarIndent +} + +// Adds a new global environment followed by a blank line to a workflow. +// Assumes a global environment does not yet exist. +// +// Returns: +// - []string: the new array of lines describing the workflow, now with the global `env:` inserted. +// - int: the row where the `env:` block was added +func addNewGlobalEnv(lines [][]byte, globalIndentation int) ([][]byte, int, error) { + envPos, err := findNewEnvPos(lines, globalIndentation) + if err != nil { + return nil, -1, err + } + + label := append(bytes.Repeat([]byte(" "), globalIndentation), []byte("env:")...) + content := [][]byte{label} + + numBlankLines := getDefaultBlockSpacing(lines, globalIndentation) + for i := 0; i < numBlankLines; i++ { + content = append(content, []byte("")) + } + + lines = slices.Insert(lines, envPos, content...) + return lines, envPos, nil +} + +// Returns the line where a new `env:` block should be inserted: right above the `jobs:` label. +func findNewEnvPos(lines [][]byte, globalIndent int) (int, error) { + jobsRegex := labelRegex("jobs", globalIndent) + for i, line := range lines { + if jobsRegex.Match(line) { + return i, nil + } + } + + return -1, sce.WithMessage(sce.ErrScorecardInternal, "Could not determine location for new environment") +} + +// Returns the "global" indentation, as defined by the indentation on the required `on:` block. +// Will equal 0 in almost all cases. +func findGlobalIndentation(lines [][]byte) (int, error) { + r := regexp.MustCompile(`^\s*on:`) + for _, line := range lines { + if r.Match(line) { + return getIndent(line), nil + } + } + + return -1, sce.WithMessage(sce.ErrScorecardInternal, "Could not determine global indentation") +} + +// Returns the indentation of the given line. The indentation is all leading whitespace and dashes. +func getIndent(line []byte) int { + return len(line) - len(bytes.TrimLeft(line, " -")) +} + +// Returns the "default" number of blank lines between blocks. +// The default is taken as the number of blank lines between the `jobs` label and the end of the preceding block. +func getDefaultBlockSpacing(lines [][]byte, globalIndent int) int { + jobsRegex := labelRegex("jobs", globalIndent) + + var jobsIdx int + var line []byte + for jobsIdx, line = range lines { + if jobsRegex.Match(line) { + break + } + } + + numBlanks := 0 + for i := jobsIdx - 1; i >= 0; i-- { + line := lines[i] + + if isBlank(line) { + numBlanks++ + } else if !isComment(line) { + // If the line is neither blank nor a comment, then we've reached the end of the previous block. + break + } + } + + return numBlanks +} + +// Returns whether the given line is a blank line (empty or only whitespace). +func isBlank(line []byte) bool { + blank := regexp.MustCompile(`^\s*$`) + return blank.Match(line) +} + +// Returns whether the given line only contains comments. +func isComment(line []byte) bool { + comment := regexp.MustCompile(`^\s*#`) + return comment.Match(line) +} + +func isBlankOrComment(line []byte) bool { + return isBlank(line) || isComment(line) +} + +// Returns whether the given line is at the same indentation level as the parent scope. +// For example, when walking through the document, parsing `job_foo`: +// +// job_foo: +// runs-on: ubuntu-latest # looping over these lines, we have +// uses: ./actions/foo # parent_indent = 2 (job_foo's indentation) +// ... # we know these lines belong to job_foo because +// ... # they all have indent = 4 +// job_bar: # this line has job_foo's indentation, so we know job_foo is done +// +// Blank lines and those containing only comments are ignored and always return false. +func isParentLevelIndent(line []byte, parentIndent int) bool { + if isBlankOrComment(line) { + return false + } + return getIndent(line) <= parentIndent +} + +func labelRegex(label string, indent int) *regexp.Regexp { + return regexp.MustCompile(fmt.Sprintf("^%s%s:", strings.Repeat(" ", indent), label)) +} + +// Returns the default indentation step adopted in the document. +// This is taken from the difference in indentation between the `jobs:` label and the first job's label. +func getDefaultIndentStep(lines [][]byte) int { + jobs := regexp.MustCompile(`^\s*jobs:`) + var jobsIndex, jobsIndent int + for i, line := range lines { + if jobs.Match(line) { + jobsIndex = i + jobsIndent = getIndent(line) + break + } + } + + jobIndent := jobsIndent + 2 // default value, should never be used + for _, line := range lines[jobsIndex+1:] { + if isBlankOrComment(line) { + continue + } + jobIndent = getIndent(line) + break + } + + return jobIndent - jobsIndent +} + +// Validates that the patch does not add any new syntax errors to the workflow. If the original workflow contains +// errors, then the patched version also might. As long as all the patch's errors match the original's, it is validated. +// +// Returns the array of new parsing errors caused by the patch. +func validatePatchedWorkflow(content []byte, originalErrs []*actionlint.Error) []*actionlint.Error { + _, patchedErrs := actionlint.Parse(content) + if len(patchedErrs) == 0 { + return []*actionlint.Error{} + } + if len(originalErrs) == 0 { + return patchedErrs + } + + normalizeMsg := func(msg string) string { + // one of the error messages contains line metadata that may legitimately change after a patch. + // Only looking at the errors' first sentence eliminates this. + return strings.Split(msg, ".")[0] + } + + var newErrs []*actionlint.Error + + o := 0 + orig := originalErrs[o] + origMsg := normalizeMsg(orig.Message) + + for _, patched := range patchedErrs { + if o == len(originalErrs) { + // no more errors in the original workflow, must be an error from our patch + newErrs = append(newErrs, patched) + continue + } + + msg := normalizeMsg(patched.Message) + if orig.Column == patched.Column && orig.Kind == patched.Kind && origMsg == msg { + // Matched error, therefore not due to our patch. + o++ + if o < len(originalErrs) { + orig = originalErrs[o] + origMsg = normalizeMsg(orig.Message) + } + } else { + newErrs = append(newErrs, patched) + } + } + + return newErrs +} + +// Returns the changes between the original and patched workflows as a unified diff (same as `git diff` or `diff -u`). +func getDiff(path string, original, patched []byte) (string, error) { + // initialize an in-memory repository + repo, err := newInMemoryRepo() + if err != nil { + return "", err + } + + // commit original workflow + originalCommit, err := commitWorkflow(path, original, repo) + if err != nil { + return "", err + } + + // commit patched workflow + patchedCommit, err := commitWorkflow(path, patched, repo) + if err != nil { + return "", err + } + + // get diff between those commits + return toUnifiedDiff(originalCommit, patchedCommit) +} + +func newInMemoryRepo() (*git.Repository, error) { + repo, err := git.Init(memory.NewStorage(), memfs.New()) + if err != nil { + return nil, fmt.Errorf("git.Init: %w", err) + } + + return repo, nil +} + +// Commits the workflow at the given path to the in-memory repository. +func commitWorkflow(path string, contents []byte, repo *git.Repository) (*object.Commit, error) { + worktree, err := repo.Worktree() + if err != nil { + return nil, fmt.Errorf("repo.Worktree: %w", err) + } + filesystem := worktree.Filesystem + + // create (or overwrite) file + df, err := filesystem.Create(path) + if err != nil { + return nil, fmt.Errorf("filesystem.Create: %w", err) + } + + _, err = df.Write(contents) + if err != nil { + return nil, fmt.Errorf("df.Write: %w", err) + } + df.Close() + + // commit file to in-memory repository + _, err = worktree.Add(path) + if err != nil { + return nil, fmt.Errorf("worktree.Add: %w", err) + } + + hash, err := worktree.Commit("x", &git.CommitOptions{}) + if err != nil { + return nil, fmt.Errorf("worktree.Commit: %w", err) + } + + commit, err := repo.CommitObject(hash) + if err != nil { + return nil, fmt.Errorf("repo.CommitObject: %w", err) + } + + return commit, nil +} + +// Returns a unified diff describing the difference between the given commits. +func toUnifiedDiff(originalCommit, patchedCommit *object.Commit) (string, error) { + patch, err := originalCommit.Patch(patchedCommit) + if err != nil { + return "", fmt.Errorf("originalCommit.Patch: %w", err) + } + builder := strings.Builder{} + err = patch.Encode(&builder) + if err != nil { + return "", fmt.Errorf("patch.Encode: %w", err) + } + + return builder.String(), nil +} diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl_test.go b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl_test.go new file mode 100644 index 00000000000..6ccff5e6a5d --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/impl_test.go @@ -0,0 +1,241 @@ +// Copyright 2024 OpenSSF Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package patch + +import ( + "context" + "fmt" + "io" + "os" + "path" + "slices" + "strings" + "testing" + + "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + "github.com/rhysd/actionlint" + + "github.com/ossf/scorecard/v5/checker" + "github.com/ossf/scorecard/v5/checks/fileparser" + "github.com/ossf/scorecard/v5/checks/raw" + mockrepo "github.com/ossf/scorecard/v5/clients/mockclients" +) + +const ( + testDir = "./testdata" +) + +func Test_patchWorkflow(t *testing.T) { + t.Parallel() + tests := []struct { + duplicates map[int]int // mark findings as duplicates of others, use same fix + name string + filePath string + }{ + { + // Extracted from real Angular fix: https://github.com/angular/angular/pull/51026/files + name: "Real Example 1", + filePath: "realExample1.yaml", + }, + { + // Inspired on a real fix: https://github.com/googleapis/google-cloud-go/pull/9011/files + name: "Real Example 2", + filePath: "realExample2.yaml", + }, + { + // Inspired from a real lit/lit fix: https://github.com/lit/lit/pull/3669/files + name: "Real Example 3", + filePath: "realExample3.yaml", + }, + { + name: "User's input is assigned to a variable before used", + filePath: "userInputAssignedToVariable.yaml", + }, + { + name: "Two incidences in different jobs", + filePath: "twoInjectionsDifferentJobs.yaml", + }, + { + name: "Two incidences in same job", + filePath: "twoInjectionsSameJob.yaml", + }, + { + name: "Two incidences in same step", + filePath: "twoInjectionsSameStep.yaml", + duplicates: map[int]int{ + 2: 1, // finding #2 is a duplicate of #1 + }, + }, + { + name: "4-spaces indentation is kept the same", + filePath: "fourSpacesIndentationExistentEnvVar.yaml", + }, + { + name: "Crazy but valid indentation is kept the same", + filePath: "crazyButValidIndentation.yaml", + }, + { + name: "Newline on EOF is kept", + filePath: "newlineOnEOF.yaml", + }, + { + name: "Ignore if user input regex is just part of a comment", + filePath: "ignorePatternInsideComments.yaml", + }, + { + name: "Reuse existent workflow level env var, if has the same name we'd give", + filePath: "reuseWorkflowLevelEnvVars.yaml", + }, + { + name: "Reuse existent workflow level env var, if it DOES NOT have the same name we'd give", + filePath: "reuseEnvVarWithDiffName.yaml", + }, + { + name: "Avoid conflict with existing envvar with same name but different value", + filePath: "envVarNameAlreadyInUse.yaml", + }, + { + name: "Avoid conflict between array variables", + filePath: "arrayVariables.yaml", + }, + // // Test currently failing because we don't look for existent env vars on smaller scopes -- job-level or step-level. + // // In this case, we're always creating a new workflow-level env var. Note that this could lead to creation of env vars shadowed + // // by the ones in smaller scope. + // // Once proper behavior is implemented, enable this test + // // { + // // name: "Reuse env var already existent on smaller scope, it converts case of same or different names", + // // inputFilepath: "reuseEnvVarSmallerScope.yaml", + // // }, + // // Test currently failing due to lack of style awareness. Currently we always add a blank line after + // // the env block. + // // Once proper behavior is implemented, enable this test. + // // { + // // name: "Keep style if file doesn't use blank lines between blocks", + // // inputFilepath: "noLineBreaksBetweenBlocks.yaml", + // // expectedFilepath: "noLineBreaksBetweenBlocks_fixed.yaml", + // // }, + } + for _, tt := range tests { + tt := tt // Re-initializing variable so it is not changed while executing the closure below + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dws := detectDangerousWorkflows(t, tt.filePath) + + inputContent, workflow, inputErrs, err := readWorkflow(tt.filePath) + if err != nil { + t.Errorf("Error reading workflow: %s", err) + } + + numFindings := len(dws) + for i, dw := range dws { + i++ // Only used for error messages, increment for legibility + + output, err := patchWorkflow(dw.File, inputContent, workflow) + if err != nil { + t.Errorf("Couldn't patch workflow for finding #%d. Error:\n%s", i, err) + } + + patchedErrs := validatePatchedWorkflow(output, inputErrs) + if len(patchedErrs) > 0 { + t.Errorf("Patched workflow for finding #%d is invalid. Error:\n%s", i, + fileparser.FormatActionlintError(patchedErrs)) + } + + if dup, ok := tt.duplicates[i]; ok { + i = dup + } + + expected := getExpected(t, tt.filePath, numFindings, i) + + if diff := cmp.Diff(expected, output); diff != "" { + t.Errorf("mismatch for finding #%d. (-want +got):\n%s", i, diff) + } + } + }) + } +} + +func readWorkflow(filePath string) ([]byte, *actionlint.Workflow, []*actionlint.Error, error) { + inputContent, err := os.ReadFile(path.Join(testDir, filePath)) + if err != nil { + return nil, nil, nil, err + } + + workflow, inputErrs := actionlint.Parse(inputContent) + if len(inputErrs) > 0 && workflow == nil { + return inputContent, nil, inputErrs, inputErrs[0] + } + + return inputContent, workflow, inputErrs, nil +} + +func getExpected(t *testing.T, filePath string, numFindings, findingIndex int) []byte { + t.Helper() + // build path to fixed version + dot := strings.LastIndex(filePath, ".") + fixedPath := filePath[:dot] + "_fixed" + if numFindings > 1 { + fixedPath = fmt.Sprintf("%s_%d", fixedPath, findingIndex) + } + fixedPath += filePath[dot:] + + content, err := os.ReadFile(path.Join(testDir, fixedPath)) + if err != nil { + t.Errorf("Couldn't read expected output file for finding #%d. Error:\n%s", findingIndex, err) + } + return content +} + +func detectDangerousWorkflows(t *testing.T, filePath string) []checker.DangerousWorkflow { + t.Helper() + ctrl := gomock.NewController(t) + mockRepoClient := mockrepo.NewMockRepoClient(ctrl) + mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return( + // Pretend the file is in the workflow directory to pass a check deep in + // raw.DangerousWorkflow + []string{path.Join(".github/workflows/", filePath)}, nil, + ) + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("./testdata/" + filePath) + }).AnyTimes() + + req := &checker.CheckRequest{ + Ctx: context.Background(), + RepoClient: mockRepoClient, + } + + dw, err := raw.DangerousWorkflow(req) + if err != nil { + t.Errorf("Error running raw.DangerousWorkflow. Error:\n%s", err) + } + + // Sort findings by position. This ensures each finding is compared to its + // respective "fixed" workflow. + slices.SortFunc(dw.Workflows, func(a, b checker.DangerousWorkflow) int { + aPos := a.File.Offset + bPos := b.File.Offset + if aPos < bPos { + return -1 + } + if aPos > bPos { + return +1 + } + return 0 + }) + + return dw.Workflows +} diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables.yaml new file mode 100644 index 00000000000..7b58f8e64be --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables.yaml @@ -0,0 +1,27 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - run: | + echo "${{ github.event.commits[0].message }}" + echo "${{ github.event.commits[1].message }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_1.yaml new file mode 100644 index 00000000000..c64c5d3f50b --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_1.yaml @@ -0,0 +1,30 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + COMMIT_MESSAGE_0: ${{ github.event.commits[0].message }} + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - run: | + echo "$COMMIT_MESSAGE_0" + echo "${{ github.event.commits[1].message }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_2.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_2.yaml new file mode 100644 index 00000000000..d0b3e17a994 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/arrayVariables_fixed_2.yaml @@ -0,0 +1,30 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + COMMIT_MESSAGE_1: ${{ github.event.commits[1].message }} + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - run: | + echo "${{ github.event.commits[0].message }}" + echo "$COMMIT_MESSAGE_1" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation.yaml new file mode 100644 index 00000000000..a3535b79e63 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation.yaml @@ -0,0 +1,37 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + on: [pull_request] + + + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + # I make no sense + + # not really + + + # some extra tabs here to check if they're kept + + - name: Check title + run: | + if [[ ! "${{ github.event.issue.title }}" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation_fixed.yaml new file mode 100644 index 00000000000..f75bd1d05b4 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/crazyButValidIndentation_fixed.yaml @@ -0,0 +1,41 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + on: [pull_request] + + + env: + ISSUE_TITLE: ${{ github.event.issue.title }} + + + jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + # I make no sense + + # not really + + + # some extra tabs here to check if they're kept + + - name: Check title + run: | + if [[ ! "$ISSUE_TITLE" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse.yaml new file mode 100644 index 00000000000..0d8d28a28da --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse.yaml @@ -0,0 +1,35 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + # existing envvar with the same name as what we'd use forces us to append a suffix. + COMMIT_MESSAGE: "this is a commit message" + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Check msg + run: | + msg="${{ github.event.head_commit.message }}" + if [[ ! $msg =~ ^.*:\ .*$ ]]; then + echo "Bad message " + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse_fixed.yaml new file mode 100644 index 00000000000..78f338477c1 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/envVarNameAlreadyInUse_fixed.yaml @@ -0,0 +1,36 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + # existing envvar with the same name as what we'd use forces us to append a suffix. + COMMIT_MESSAGE: "this is a commit message" + COMMIT_MESSAGE_1: ${{ github.event.head_commit.message }} + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Check msg + run: | + msg="$COMMIT_MESSAGE_1" + if [[ ! $msg =~ ^.*:\ .*$ ]]; then + echo "Bad message " + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar.yaml new file mode 100644 index 00000000000..42fc98f9e53 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar.yaml @@ -0,0 +1,27 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + SUPER_RELEVANT: "bla" + +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: | + if [[ ! "${{ github.event.issue.title }}" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar_fixed.yaml new file mode 100644 index 00000000000..c003ca4058f --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/fourSpacesIndentationExistentEnvVar_fixed.yaml @@ -0,0 +1,28 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + SUPER_RELEVANT: "bla" + ISSUE_TITLE: ${{ github.event.issue.title }} + +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: | + if [[ ! "$ISSUE_TITLE" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/ignorePatternInsideComments.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/ignorePatternInsideComments.yaml new file mode 100644 index 00000000000..45e4605ea8a --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/ignorePatternInsideComments.yaml @@ -0,0 +1,24 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: issue_comment + +jobs: + issue_commented: + # This job only runs for issue comments + name: Issue comment + if: ${{ !github.event.issue.pull_request }} + runs-on: ubuntu-latest + steps: + - run: | + echo "Comment on issue #${{ github.event.issue.number }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF.yaml new file mode 100644 index 00000000000..db658e33d8d --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF.yaml @@ -0,0 +1,26 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Check title + run: | + if [[ ! "${{ github.event.issue.title }}" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi +# Extra line below, to test if we keep the newline on EOF diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF_fixed.yaml new file mode 100644 index 00000000000..d9e7efae69d --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/newlineOnEOF_fixed.yaml @@ -0,0 +1,29 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + ISSUE_TITLE: ${{ github.event.issue.title }} + +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: Check title + run: | + if [[ ! "$ISSUE_TITLE" =~ ^.*:\ .*$ ]]; then + echo "Bad issue title" + exit 1 + fi +# Extra line below, to test if we keep the newline on EOF diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks.yaml new file mode 100644 index 00000000000..fc8d44c3d28 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks.yaml @@ -0,0 +1,24 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: The anti-blanklines workflow +on: + issue: +permissions: + contents: read +jobs: + simple-job: + steps: + - name: As simple as it can be + run: | + echo "${{ github.event.comment.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks_fixed.yaml new file mode 100644 index 00000000000..04097893113 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/noLineBreaksBetweenBlocks_fixed.yaml @@ -0,0 +1,26 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: The anti-blanklines workflow +on: + issue: +permissions: + contents: read +env: + COMMENT_BODY: "${{ github.event.comment.body }}" +jobs: + simple-job: + steps: + - name: As simple as it can be + run: | + echo "$COMMENT_BODY" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1.yaml new file mode 100644 index 00000000000..843d0c7118a --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1.yaml @@ -0,0 +1,58 @@ +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + +permissions: read-all + +jobs: + benchmark-compare: + runs-on: ubuntu-latest + if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/benchmark-compare ')}} + steps: + - uses: TheModdingInquisition/actions-team-membership@a69636a92bc927f32c3910baac06bacc949c984c # v1.0 + with: + team: 'team' + organization: angular + token: ${{secrets.BENCHMARK_COMPARE_MEMBERSHIP_GITHUB_TOKEN}} + exit: true + # Indicate that the benchmark command was received. + - uses: peter-evans/create-or-update-comment@c6c9a1a66007646a28c153e2a8580a5bad27bcfa # v3 + with: + comment-id: ${{github.event.comment.id}} + token: '${{secrets.BENCHMARK_POST_RESULTS_GITHUB_TOKEN}}' + reactions: 'rocket' + - uses: alessbell/pull-request-comment-branch@aad01d65d6982b8eacabed5e9a684cd8ceb98da6 # v1.1 + id: comment-branch + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + # Specify repository as the PR branch might be from a fork. + repository: ${{steps.comment-branch.outputs.head_owner}}/${{steps.comment-branch.outputs.head_repo}} + # Checkout the pull request and assume it being trusted given we've checked + # that the action was triggered by a team member. + ref: ${{steps.comment-branch.outputs.head_ref}} + - uses: ./.github/actions/yarn-install + - uses: angular/dev-infra/github-actions/setup-bazel-remote-exec@97cb376945b07e37671e3b38ee727ff00dc1394b + with: + bazelrc: ./.bazelrc.user + + - name: Preparing benchmark for GitHub action + id: info + run: yarn benchmarks prepare-for-github-action "${{github.event.comment.body}}" + + - run: yarn benchmarks run-compare ${{steps.info.outputs.compareSha}} ${{steps.info.outputs.benchmarkTarget}} + id: benchmark + name: Running benchmark + + - uses: peter-evans/create-or-update-comment@c6c9a1a66007646a28c153e2a8580a5bad27bcfa # v3 + with: + issue-number: ${{github.event.issue.number}} + token: '${{secrets.BENCHMARK_POST_RESULTS_GITHUB_TOKEN}}' + body: | + ## Benchmark Test Results + **Test**: `${{steps.info.outputs.benchmarkTarget}}` + ### PR (${{steps.info.outputs.prHeadSha}}) + ${{steps.benchmark.outputs.workingStageResultsText}} + ### Compare Ref (${{steps.info.outputs.compareSha}}) + ${{steps.benchmark.outputs.comparisonResultsText}} \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1_fixed.yaml new file mode 100644 index 00000000000..cced3454af3 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample1_fixed.yaml @@ -0,0 +1,61 @@ +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + +permissions: read-all + +env: + COMMENT_BODY: ${{ github.event.comment.body }} + +jobs: + benchmark-compare: + runs-on: ubuntu-latest + if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/benchmark-compare ')}} + steps: + - uses: TheModdingInquisition/actions-team-membership@a69636a92bc927f32c3910baac06bacc949c984c # v1.0 + with: + team: 'team' + organization: angular + token: ${{secrets.BENCHMARK_COMPARE_MEMBERSHIP_GITHUB_TOKEN}} + exit: true + # Indicate that the benchmark command was received. + - uses: peter-evans/create-or-update-comment@c6c9a1a66007646a28c153e2a8580a5bad27bcfa # v3 + with: + comment-id: ${{github.event.comment.id}} + token: '${{secrets.BENCHMARK_POST_RESULTS_GITHUB_TOKEN}}' + reactions: 'rocket' + - uses: alessbell/pull-request-comment-branch@aad01d65d6982b8eacabed5e9a684cd8ceb98da6 # v1.1 + id: comment-branch + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + # Specify repository as the PR branch might be from a fork. + repository: ${{steps.comment-branch.outputs.head_owner}}/${{steps.comment-branch.outputs.head_repo}} + # Checkout the pull request and assume it being trusted given we've checked + # that the action was triggered by a team member. + ref: ${{steps.comment-branch.outputs.head_ref}} + - uses: ./.github/actions/yarn-install + - uses: angular/dev-infra/github-actions/setup-bazel-remote-exec@97cb376945b07e37671e3b38ee727ff00dc1394b + with: + bazelrc: ./.bazelrc.user + + - name: Preparing benchmark for GitHub action + id: info + run: yarn benchmarks prepare-for-github-action "$COMMENT_BODY" + + - run: yarn benchmarks run-compare ${{steps.info.outputs.compareSha}} ${{steps.info.outputs.benchmarkTarget}} + id: benchmark + name: Running benchmark + + - uses: peter-evans/create-or-update-comment@c6c9a1a66007646a28c153e2a8580a5bad27bcfa # v3 + with: + issue-number: ${{github.event.issue.number}} + token: '${{secrets.BENCHMARK_POST_RESULTS_GITHUB_TOKEN}}' + body: | + ## Benchmark Test Results + **Test**: `${{steps.info.outputs.benchmarkTarget}}` + ### PR (${{steps.info.outputs.prHeadSha}}) + ${{steps.benchmark.outputs.workingStageResultsText}} + ### Compare Ref (${{steps.info.outputs.compareSha}}) + ${{steps.benchmark.outputs.comparisonResultsText}} \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2.yaml new file mode 100644 index 00000000000..8c91b3653cb --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2.yaml @@ -0,0 +1,99 @@ +--- +name: apidiff + +on: + pull_request: + +permissions: + contents: read + pull-requests: write + +jobs: + scan_changes: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + ref: main + - name: Get main commit + id: main + run: echo "hash=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: '1.21.x' + - name: Get changed directories + id: changed_dirs + # Ignore changes to the internal and root directories. + # Ignore added files with --diff-filter=a. + run: | + dirs=$(go run ./internal/actions/cmd/changefinder -q --diff-filter=a) + if [ -z "$dirs" ] + then + echo "skip=1" >> $GITHUB_OUTPUT + echo "No changes worth diffing!" + else + for d in $dirs; do list=${list},\"${d}\"; done + echo "changed={\"changed\":[${list#,}]}" >> $GITHUB_OUTPUT + echo "skip=" >> $GITHUB_OUTPUT + fi + outputs: + changed_dirs: ${{ steps.changed_dirs.outputs.changed }} + skip: ${{ steps.changed_dirs.outputs.skip }} + apidiff: + needs: scan_changes + runs-on: ubuntu-latest + if: "!needs.scan_changes.outputs.skip && !contains(github.event.pull_request.labels.*.name, 'breaking change allowed')" + continue-on-error: true + strategy: + matrix: ${{ fromJson(needs.scan_changes.outputs.changed_dirs) }} + steps: + - uses: actions/setup-go@v4 + with: + go-version: '1.21.x' + - name: Install latest apidiff + run: go install golang.org/x/exp/cmd/apidiff@latest + - uses: actions/checkout@v3 + with: + ref: main + - name: Create baseline + id: baseline + run: | + export CHANGED=${{ matrix.changed }} + echo pkg="${CHANGED//\//_}_pkg.main" >> $GITHUB_OUTPUT + - name: Create Go package baseline + run: cd ${{ matrix.changed }} && apidiff -m -w ${{ steps.baseline.outputs.pkg }} . + - name: Upload baseline package data + uses: actions/upload-artifact@v3 + with: + name: ${{ steps.baseline.outputs.pkg }} + path: ${{ matrix.changed }}/${{ steps.baseline.outputs.pkg }} + retention-days: 1 + - uses: actions/checkout@v3 + - name: Download baseline package data + uses: actions/download-artifact@v3 + with: + name: ${{ steps.baseline.outputs.pkg }} + path: ${{ matrix.changed }} + - name: Compare regenerated code to baseline + # Only ignore Go interface additions when the PR is from OwlBot as it is + # likely a new method added to the gRPC client stub interface, which is + # non-breaking. + run: | + cd ${{ matrix.changed }} && apidiff -m -incompatible ${{ steps.baseline.outputs.pkg }} . > diff.txt + if [[ ${{ github.event.pull_request.head.ref }} == owl-bot-copy ]]; then + sed -i '/: added/d' ./diff.txt + fi + cat diff.txt && ! [ -s diff.txt ] + + - name: Add breaking change label + if: ${{ failure() && !github.event.pull_request.head.repo.fork }} + uses: actions/github-script@v6 + with: + script: | + github.rest.issues.addLabels({ + issue_number: ${{ github.event.number }}, + owner: context.repo.owner, + repo: context.repo.repo, + labels: ['breaking change'] + }) \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2_fixed.yaml new file mode 100644 index 00000000000..b9958c80215 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample2_fixed.yaml @@ -0,0 +1,102 @@ +--- +name: apidiff + +on: + pull_request: + +permissions: + contents: read + pull-requests: write + +env: + PR_HEAD_REF: ${{ github.event.pull_request.head.ref }} + +jobs: + scan_changes: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + ref: main + - name: Get main commit + id: main + run: echo "hash=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: '1.21.x' + - name: Get changed directories + id: changed_dirs + # Ignore changes to the internal and root directories. + # Ignore added files with --diff-filter=a. + run: | + dirs=$(go run ./internal/actions/cmd/changefinder -q --diff-filter=a) + if [ -z "$dirs" ] + then + echo "skip=1" >> $GITHUB_OUTPUT + echo "No changes worth diffing!" + else + for d in $dirs; do list=${list},\"${d}\"; done + echo "changed={\"changed\":[${list#,}]}" >> $GITHUB_OUTPUT + echo "skip=" >> $GITHUB_OUTPUT + fi + outputs: + changed_dirs: ${{ steps.changed_dirs.outputs.changed }} + skip: ${{ steps.changed_dirs.outputs.skip }} + apidiff: + needs: scan_changes + runs-on: ubuntu-latest + if: "!needs.scan_changes.outputs.skip && !contains(github.event.pull_request.labels.*.name, 'breaking change allowed')" + continue-on-error: true + strategy: + matrix: ${{ fromJson(needs.scan_changes.outputs.changed_dirs) }} + steps: + - uses: actions/setup-go@v4 + with: + go-version: '1.21.x' + - name: Install latest apidiff + run: go install golang.org/x/exp/cmd/apidiff@latest + - uses: actions/checkout@v3 + with: + ref: main + - name: Create baseline + id: baseline + run: | + export CHANGED=${{ matrix.changed }} + echo pkg="${CHANGED//\//_}_pkg.main" >> $GITHUB_OUTPUT + - name: Create Go package baseline + run: cd ${{ matrix.changed }} && apidiff -m -w ${{ steps.baseline.outputs.pkg }} . + - name: Upload baseline package data + uses: actions/upload-artifact@v3 + with: + name: ${{ steps.baseline.outputs.pkg }} + path: ${{ matrix.changed }}/${{ steps.baseline.outputs.pkg }} + retention-days: 1 + - uses: actions/checkout@v3 + - name: Download baseline package data + uses: actions/download-artifact@v3 + with: + name: ${{ steps.baseline.outputs.pkg }} + path: ${{ matrix.changed }} + - name: Compare regenerated code to baseline + # Only ignore Go interface additions when the PR is from OwlBot as it is + # likely a new method added to the gRPC client stub interface, which is + # non-breaking. + run: | + cd ${{ matrix.changed }} && apidiff -m -incompatible ${{ steps.baseline.outputs.pkg }} . > diff.txt + if [[ $PR_HEAD_REF == owl-bot-copy ]]; then + sed -i '/: added/d' ./diff.txt + fi + cat diff.txt && ! [ -s diff.txt ] + + - name: Add breaking change label + if: ${{ failure() && !github.event.pull_request.head.repo.fork }} + uses: actions/github-script@v6 + with: + script: | + github.rest.issues.addLabels({ + issue_number: ${{ github.event.number }}, + owner: context.repo.owner, + repo: context.repo.repo, + labels: ['breaking change'] + }) \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3.yaml new file mode 100644 index 00000000000..8583efef84b --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3.yaml @@ -0,0 +1,44 @@ +name: Generate Release Image + +on: + pull_request: + paths: + - '**/CHANGELOG.md' + push: + paths: + - '**/CHANGELOG.md' + +jobs: + release-image: + if: github.repository == 'lit/lit' + name: Generate Release Image + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-node@v2 + with: + node-version: 16 + cache: 'npm' + cache-dependency-path: package-lock.json + - uses: google/wireit@setup-github-actions-caching/v1 + - name: NPM install + run: npm ci + - name: Build release-image script + working-directory: packages/internal-scripts + run: npm run build + + - name: Create release image + run: | + # Store the PR body contents containing the changelog in 'release.md' + cat <<'EOF' > release.md + ${{ github.event.pull_request.body }} + EOF + # Only render the pull request content including and after the "# + # Releases" heading. + node_modules/.bin/release-image -m <(sed -n '/# Releases/,$p' release.md) + + - name: Upload artifact + uses: actions/upload-artifact@v3 + with: + name: releaseimage + path: release.png \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3_fixed.yaml new file mode 100644 index 00000000000..56c434770f1 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/realExample3_fixed.yaml @@ -0,0 +1,47 @@ +name: Generate Release Image + +on: + pull_request: + paths: + - '**/CHANGELOG.md' + push: + paths: + - '**/CHANGELOG.md' + +env: + PR_BODY: ${{ github.event.pull_request.body }} + +jobs: + release-image: + if: github.repository == 'lit/lit' + name: Generate Release Image + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-node@v2 + with: + node-version: 16 + cache: 'npm' + cache-dependency-path: package-lock.json + - uses: google/wireit@setup-github-actions-caching/v1 + - name: NPM install + run: npm ci + - name: Build release-image script + working-directory: packages/internal-scripts + run: npm run build + + - name: Create release image + run: | + # Store the PR body contents containing the changelog in 'release.md' + cat <<'EOF' > release.md + $PR_BODY + EOF + # Only render the pull request content including and after the "# + # Releases" heading. + node_modules/.bin/release-image -m <(sed -n '/# Releases/,$p' release.md) + + - name: Upload artifact + uses: actions/upload-artifact@v3 + with: + name: releaseimage + path: release.png \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope.yaml new file mode 100644 index 00000000000..351b856fca9 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope.yaml @@ -0,0 +1,53 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-job-level-env-vars: + env: + # Safe but unnused env var. Same name that our script would use + ISSUE_BODY: "${{ github.event.issue.body }}" + + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + PULL_REQ_BODY: ${{ github.event.pull_request.body }} + steps: + - run: | + echo "${{ github.event.issue.body }}" + - run: | + echo "${{ github.event.pull_request.body }}" + + using-step-level-env-vars: + steps: + - name: the step + env: + # Safe but unnused env var. Same name that our script would use + HEAD_COMMIT_MESSAGE: "${{ github.event.head_commit.message }}" + + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + PULL_REQ_TITLE: ${{ github.event.pull_request.title }} + run: | + echo "${{ github.event.head_commit.message }}" + echo "${{ github.event.pull_request.title }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope_fixed.yaml new file mode 100644 index 00000000000..7fa12a97047 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarSmallerScope_fixed.yaml @@ -0,0 +1,53 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-job-level-env-vars: + env: + # Safe but unnused env var. Same name that our script would use + ISSUE_BODY: "${{ github.event.issue.body }}" + + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + PULL_REQ_BODY: ${{ github.event.pull_request.body }} + steps: + - run: | + echo "$ISSUE_BODY" + - run: | + echo "$PULL_REQ_BODY" + + using-step-level-env-vars: + steps: + - name: the step + env: + # Safe but unnused env var. Same name that our script would use + HEAD_COMMIT_MESSAGE: "${{ github.event.head_commit.message }}" + + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + PULL_REQ_TITLE: ${{ github.event.pull_request.title }} + run: | + echo "$HEAD_COMMIT_MESSAGE" + echo "$PULL_REQ_TITLE" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName.yaml new file mode 100644 index 00000000000..2e2fa7c4fa5 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName.yaml @@ -0,0 +1,31 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + +permissions: read-all + +env: + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + TITLE: ${{github.event.issue.title}} + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "${{ github.event.issue.title }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName_fixed.yaml new file mode 100644 index 00000000000..051ea87be2c --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseEnvVarWithDiffName_fixed.yaml @@ -0,0 +1,31 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +on: + issue_comment: + types: [created] + +permissions: read-all + +env: + # Safe but unnused env var. Different name than the one our script would use. + # Ideally we should keep the name as it is + TITLE: ${{github.event.issue.title}} + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "$TITLE" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars.yaml new file mode 100644 index 00000000000..0b5a6dddb51 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars.yaml @@ -0,0 +1,56 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +# Env block intentionally not placed right above the "jobs" block, where our script usually places it +env: + # Existent but non-related env var + ISSUE_NUMBER: ${{github.event.issue.number}} + + # Safe but unnused env var. Same name that our script would use. No spaces inside brackets + ISSUE_COMMENT_BODY: "${{github.event.issue_comment.comment.body}}" + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "$ISSUE_NUMBER" + echo "${{ github.event.issue_comment.comment.body }}" + + # content orinally not present in any env var. + # This same content will be used again and should reuse created env var + - run: | + echo "${{ github.event.issue.body }}" + + using-job-level-env-vars: + env: + # Existent but non-related env var + NUM_COMMENTS: ${{ github.event.issue.comments }} + steps: + - run: | + echo "$NUM_COMMENTS" + + # Same variable that already was used on previous job. They should reuse the same workflow-level env var + - run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_1.yaml new file mode 100644 index 00000000000..71a2e7f783d --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_1.yaml @@ -0,0 +1,56 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +# Env block intentionally not placed right above the "jobs" block, where our script usually places it +env: + # Existent but non-related env var + ISSUE_NUMBER: ${{github.event.issue.number}} + + # Safe but unnused env var. Same name that our script would use. No spaces inside brackets + ISSUE_COMMENT_BODY: "${{github.event.issue_comment.comment.body}}" + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "$ISSUE_NUMBER" + echo "$ISSUE_COMMENT_BODY" + + # content orinally not present in any env var. + # This same content will be used again and should reuse created env var + - run: | + echo "${{ github.event.issue.body }}" + + using-job-level-env-vars: + env: + # Existent but non-related env var + NUM_COMMENTS: ${{ github.event.issue.comments }} + steps: + - run: | + echo "$NUM_COMMENTS" + + # Same variable that already was used on previous job. They should reuse the same workflow-level env var + - run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_2.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_2.yaml new file mode 100644 index 00000000000..ffc3aedec57 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_2.yaml @@ -0,0 +1,57 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +# Env block intentionally not placed right above the "jobs" block, where our script usually places it +env: + # Existent but non-related env var + ISSUE_NUMBER: ${{github.event.issue.number}} + + # Safe but unnused env var. Same name that our script would use. No spaces inside brackets + ISSUE_COMMENT_BODY: "${{github.event.issue_comment.comment.body}}" + ISSUE_BODY: ${{ github.event.issue.body }} + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "$ISSUE_NUMBER" + echo "${{ github.event.issue_comment.comment.body }}" + + # content orinally not present in any env var. + # This same content will be used again and should reuse created env var + - run: | + echo "$ISSUE_BODY" + + using-job-level-env-vars: + env: + # Existent but non-related env var + NUM_COMMENTS: ${{ github.event.issue.comments }} + steps: + - run: | + echo "$NUM_COMMENTS" + + # Same variable that already was used on previous job. They should reuse the same workflow-level env var + - run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_3.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_3.yaml new file mode 100644 index 00000000000..e0dc33f2b80 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/reuseWorkflowLevelEnvVars_fixed_3.yaml @@ -0,0 +1,57 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +name: Run benchmark comparison + +# Env block intentionally not placed right above the "jobs" block, where our script usually places it +env: + # Existent but non-related env var + ISSUE_NUMBER: ${{github.event.issue.number}} + + # Safe but unnused env var. Same name that our script would use. No spaces inside brackets + ISSUE_COMMENT_BODY: "${{github.event.issue_comment.comment.body}}" + ISSUE_BODY: ${{ github.event.issue.body }} + +on: + issue_comment: + types: [created] + issue: + types: [created] + pull_request: + types: [created] + +permissions: read-all + +jobs: + using-workflow-level-env-vars: + steps: + - run: | + echo "$ISSUE_NUMBER" + echo "${{ github.event.issue_comment.comment.body }}" + + # content orinally not present in any env var. + # This same content will be used again and should reuse created env var + - run: | + echo "${{ github.event.issue.body }}" + + using-job-level-env-vars: + env: + # Existent but non-related env var + NUM_COMMENTS: ${{ github.event.issue.comments }} + steps: + - run: | + echo "$NUM_COMMENTS" + + # Same variable that already was used on previous job. They should reuse the same workflow-level env var + - run: | + echo "$ISSUE_BODY" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs.yaml new file mode 100644 index 00000000000..05828819511 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs.yaml @@ -0,0 +1,28 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + issue: + +jobs: + fascinating-job: + steps: + - name: it runs like magic + run: | + echo "${{ github.event.issue.title }}" + + incredible-other-job: + steps: + - name: absolutely outstanding, safe as nothing else + run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_1.yaml new file mode 100644 index 00000000000..38ee58ad65b --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_1.yaml @@ -0,0 +1,31 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + issue: + +env: + ISSUE_TITLE: ${{ github.event.issue.title }} + +jobs: + fascinating-job: + steps: + - name: it runs like magic + run: | + echo "$ISSUE_TITLE" + + incredible-other-job: + steps: + - name: absolutely outstanding, safe as nothing else + run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_2.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_2.yaml new file mode 100644 index 00000000000..b7ae07c6769 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsDifferentJobs_fixed_2.yaml @@ -0,0 +1,31 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + issue: + +env: + ISSUE_BODY: ${{ github.event.issue.body }} + +jobs: + fascinating-job: + steps: + - name: it runs like magic + run: | + echo "${{ github.event.issue.title }}" + + incredible-other-job: + steps: + - name: absolutely outstanding, safe as nothing else + run: | + echo "$ISSUE_BODY" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob.yaml new file mode 100644 index 00000000000..c6c8281cb71 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob.yaml @@ -0,0 +1,26 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + discussion: + types: [created] + +jobs: + really-complete-job: + steps: + - name: it's only the beginning + run: | + echo "${{ github.event.issue.title }}" + - name: ok, now we're talking + run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_1.yaml new file mode 100644 index 00000000000..bf90d068d04 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_1.yaml @@ -0,0 +1,29 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + discussion: + types: [created] + +env: + ISSUE_TITLE: ${{ github.event.issue.title }} + +jobs: + really-complete-job: + steps: + - name: it's only the beginning + run: | + echo "$ISSUE_TITLE" + - name: ok, now we're talking + run: | + echo "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_2.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_2.yaml new file mode 100644 index 00000000000..dea3256ee38 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameJob_fixed_2.yaml @@ -0,0 +1,29 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + discussion: + types: [created] + +env: + ISSUE_BODY: ${{ github.event.issue.body }} + +jobs: + really-complete-job: + steps: + - name: it's only the beginning + run: | + echo "${{ github.event.issue.title }}" + - name: ok, now we're talking + run: | + echo "$ISSUE_BODY" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep.yaml new file mode 100644 index 00000000000..3de1f7a96db --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep.yaml @@ -0,0 +1,26 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + fork: + issue_comment: + types: [created, edited] + +jobs: + solution-to-all-repo-problems: + steps: + - name: where things are done + run: | + echo "${{ github.event.issue.title }}" + echo "${{ github.event.issue.title }}" + mkdir "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_1.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_1.yaml new file mode 100644 index 00000000000..41ff00d0744 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_1.yaml @@ -0,0 +1,29 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + fork: + issue_comment: + types: [created, edited] + +env: + ISSUE_TITLE: ${{ github.event.issue.title }} + +jobs: + solution-to-all-repo-problems: + steps: + - name: where things are done + run: | + echo "$ISSUE_TITLE" + echo "$ISSUE_TITLE" + mkdir "${{ github.event.issue.body }}" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_3.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_3.yaml new file mode 100644 index 00000000000..cf61c7ca06b --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/twoInjectionsSameStep_fixed_3.yaml @@ -0,0 +1,29 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: + fork: + issue_comment: + types: [created, edited] + +env: + ISSUE_BODY: ${{ github.event.issue.body }} + +jobs: + solution-to-all-repo-problems: + steps: + - name: where things are done + run: | + echo "${{ github.event.issue.title }}" + echo "${{ github.event.issue.title }}" + mkdir "$ISSUE_BODY" \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable.yaml new file mode 100644 index 00000000000..6a107d155f6 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable.yaml @@ -0,0 +1,31 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Check msg + run: | + msg="${{ github.event.commits[0].message }}" + if [[ ! $msg =~ ^.*:\ .*$ ]]; then + echo "Bad message " + exit 1 + fi \ No newline at end of file diff --git a/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable_fixed.yaml b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable_fixed.yaml new file mode 100644 index 00000000000..c016472bb19 --- /dev/null +++ b/probes/hasDangerousWorkflowScriptInjection/internal/patch/testdata/userInputAssignedToVariable_fixed.yaml @@ -0,0 +1,34 @@ +# Copyright 2024 OpenSSF Scorecard Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +on: [pull_request] + +env: + COMMIT_MESSAGE_0: ${{ github.event.commits[0].message }} + +jobs: + build: + name: Build and test + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Check msg + run: | + msg="$COMMIT_MESSAGE_0" + if [[ ! $msg =~ ^.*:\ .*$ ]]; then + echo "Bad message " + exit 1 + fi \ No newline at end of file