Skip to content

Commit

Permalink
🐛 Github workflow steps run on Windows should default to pwsh as its …
Browse files Browse the repository at this point in the history
…shell (#877)

* Github workflow steps run on Windows should default to pwsh as its shell

* Style change from PR feedback

* Fixing linter error

* MR feedback: simplifying code

* Moving consts to top of file

Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
Co-authored-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Co-authored-by: Naveen <172697+naveensrinivasan@users.noreply.github.com>
  • Loading branch information
4 people committed Sep 7, 2021
1 parent a3d63bf commit 1c7ba79
Show file tree
Hide file tree
Showing 15 changed files with 559 additions and 29 deletions.
164 changes: 135 additions & 29 deletions checks/pinned_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,72 @@ import (
// CheckPinnedDependencies is the registered name for FrozenDeps.
const CheckPinnedDependencies = "Pinned-Dependencies"

// defaultShellNonWindows is the default shell used for GitHub workflow actions for Linux and Mac.
const defaultShellNonWindows = "bash"

// defaultShellWindows is the default shell used for GitHub workflow actions for Windows.
const defaultShellWindows = "pwsh"

// Structure for workflow config.
// We only declare the fields we need.
// Github workflows format: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
type gitHubActionWorkflowConfig struct {
// nolint: govet
Jobs map[string]struct {
Name string `yaml:"name"`
Steps []struct {
Name string `yaml:"name"`
ID string `yaml:"id"`
Uses stringWithLine `yaml:"uses"`
Shell string `yaml:"shell"`
Run string `yaml:"run"`
}
Defaults struct {
Run struct {
Shell string `yaml:"shell"`
} `yaml:"run"`
} `yaml:"defaults"`
}
Jobs map[string]gitHubActionWorkflowJob
Name string `yaml:"name"`
}

// A Github Action Workflow Job.
// We only declare the fields we need.
// Github workflows format: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
type gitHubActionWorkflowJob struct {
Name string `yaml:"name"`
Steps []gitHubActionWorkflowStep `yaml:"steps"`
Defaults struct {
Run struct {
Shell string `yaml:"shell"`
} `yaml:"run"`
} `yaml:"defaults"`
RunsOn stringOrSlice `yaml:"runs-on"`
Strategy struct {
Matrix struct {
Os []string `yaml:"os"`
} `yaml:"matrix"`
} `yaml:"strategy"`
}

// A Github Action Workflow Step.
// We only declare the fields we need.
// Github workflows format: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions
type gitHubActionWorkflowStep struct {
Name string `yaml:"name"`
ID string `yaml:"id"`
Shell string `yaml:"shell"`
Run string `yaml:"run"`
If string `yaml:"if"`
Uses stringWithLine `yaml:"uses"`
}

// stringOrSlice is for fields that can be a single string or a slice of strings. If the field is a single string,
// this value will be a slice with a single string item.
type stringOrSlice []string

func (s *stringOrSlice) UnmarshalYAML(value *yaml.Node) error {
var stringSlice []string
err := value.Decode(&stringSlice)
if err == nil {
*s = stringSlice
return nil
}
var single string
err = value.Decode(&single)
if err != nil {
//nolint:wrapcheck
return sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("error decoding stringOrSlice Value: %v", err))
}
*s = []string{single}
return nil
}

// stringWithLine is for when you want to keep track of the line number that the string came from.
type stringWithLine struct {
Value string
Expand Down Expand Up @@ -460,32 +503,27 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by

githubVarRegex := regexp.MustCompile(`{{[^{}]*}}`)
validated := true
// https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell.
defaultShell := "bash"
scriptContent := ""
for _, job := range workflow.Jobs {
if job.Defaults.Run.Shell != "" {
defaultShell = job.Defaults.Run.Shell
}

job := job
for _, step := range job.Steps {
step := step
if step.Run == "" {
continue
}

shell := defaultShell
if step.Shell != "" {
shell = step.Shell
}

// https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstepsrun.
// Skip unsupported shells. We don't support Windows shells.
shell, err := getShellForStep(&step, &job)
if err != nil {
return false, err
}
// Skip unsupported shells. We don't support Windows shells or some Unix shells.
if !isSupportedShell(shell) {
continue
}

run := step.Run
// We replace the `${{ github.variable }}` to avoid shell parising failures.
// We replace the `${{ github.variable }}` to avoid shell parsing failures.
script := githubVarRegex.ReplaceAll([]byte(run), []byte("GITHUB_REDACTED_VAR"))
scriptContent = fmt.Sprintf("%v\n%v", scriptContent, string(script))
}
Expand All @@ -502,6 +540,74 @@ func validateGitHubWorkflowIsFreeOfInsecureDownloads(pathfn string, content []by
return true, nil
}

// The only OS that this job runs on is Windows.
func jobAlwaysRunsOnWindows(job *gitHubActionWorkflowJob) bool {
var jobOses []string
// The 'runs-on' field either lists the OS'es directly, or it can have an expression '${{ matrix.os }}' which
// is where the OS'es are actually listed.
if len(job.RunsOn) == 1 && strings.Contains(job.RunsOn[0], "matrix.os") {
jobOses = job.Strategy.Matrix.Os
} else {
jobOses = job.RunsOn
}
for _, os := range jobOses {
if !strings.HasPrefix(strings.ToLower(os), "windows-") {
return false
}
}
return true
}

// getShellForStep returns the shell that is used to run the given step.
func getShellForStep(step *gitHubActionWorkflowStep, job *gitHubActionWorkflowJob) (string, error) {
// https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#using-a-specific-shell.
if step.Shell != "" {
return step.Shell, nil
}
if job.Defaults.Run.Shell != "" {
return job.Defaults.Run.Shell, nil
}

isStepWindows, err := isStepWindows(step)
if err != nil {
return "", err
}
if isStepWindows {
return defaultShellWindows, nil
}

if jobAlwaysRunsOnWindows(job) {
return defaultShellWindows, nil
}

return defaultShellNonWindows, nil
}

// isStepWindows returns true if the step will be run on Windows.
func isStepWindows(step *gitHubActionWorkflowStep) (bool, error) {
windowsRegexes := []string{
// Looking for "if: runner.os == 'Windows'" (and variants)
`(?i)runner\.os\s*==\s*['"]windows['"]`,
// Looking for "if: ${{ startsWith(runner.os, 'Windows') }}" (and variants)
`(?i)\$\{\{\s*startsWith\(runner\.os,\s*['"]windows['"]\)`,
// Looking for "if: matrix.os == 'windows-2019'" (and variants)
`(?i)matrix\.os\s*==\s*['"]windows-`,
}

for _, windowsRegex := range windowsRegexes {
matches, err := regexp.MatchString(windowsRegex, step.If)
if err != nil {
//nolint:wrapcheck
return false, sce.Create(sce.ErrScorecardInternal, fmt.Sprintf("error matching Windows regex: %v", err))
}
if matches {
return true, nil
}
}

return false, nil
}

// Check pinning of github actions in workflows.
func isGitHubActionsWorkflowPinned(c *checker.CheckRequest) (int, error) {
var r pinnedResult
Expand Down
118 changes: 118 additions & 0 deletions checks/pinned_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"gopkg.in/yaml.v3"

"github.com/ossf/scorecard/v2/checker"
scut "github.com/ossf/scorecard/v2/utests"
)
Expand Down Expand Up @@ -642,3 +645,118 @@ func TestGitHubWorkflowUsesLineNumber(t *testing.T) {
})
}
}

func TestGitHubWorkflowShell(t *testing.T) {
t.Parallel()

repeatItem := func(item string, count int) []string {
ret := make([]string, 0, count)
for i := 0; i < count; i++ {
ret = append(ret, item)
}
return ret
}

tests := []struct {
name string
filename string
// The shells used in each step, listed in order that the steps are listed in the file
expectedShells []string
}{
{
name: "all windows, shell specified in step",
filename: "testdata/github-workflow-shells-all-windows-bash.yaml",
expectedShells: []string{"bash"},
},
{
name: "all windows, OSes listed in matrix.os",
filename: "testdata/github-workflow-shells-all-windows-matrix.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "all windows",
filename: "testdata/github-workflow-shells-all-windows.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "macOS defaults to bash",
filename: "testdata/github-workflow-shells-default-macos.yaml",
expectedShells: []string{"bash"},
},
{
name: "ubuntu defaults to bash",
filename: "testdata/github-workflow-shells-default-ubuntu.yaml",
expectedShells: []string{"bash"},
},
{
name: "windows defaults to pwsh",
filename: "testdata/github-workflow-shells-default-windows.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "windows specified in 'if'",
filename: "testdata/github-workflow-shells-runner-windows-ubuntu.yaml",
expectedShells: append(repeatItem("pwsh", 7), repeatItem("bash", 4)...),
},
{
name: "shell specified in job and step",
filename: "testdata/github-workflow-shells-specified-job-step.yaml",
expectedShells: []string{"bash"},
},
{
name: "windows, shell specified in job",
filename: "testdata/github-workflow-shells-specified-job-windows.yaml",
expectedShells: []string{"bash"},
},
{
name: "shell specified in job",
filename: "testdata/github-workflow-shells-specified-job.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "shell specified in step",
filename: "testdata/github-workflow-shells-speficied-step.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "different shells in each step",
filename: "testdata/github-workflow-shells-two-shells.yaml",
expectedShells: []string{"bash", "pwsh"},
},
{
name: "windows step, bash specified",
filename: "testdata/github-workflow-shells-windows-bash.yaml",
expectedShells: []string{"bash", "bash"},
},
}
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()
content, err := ioutil.ReadFile(tt.filename)
if err != nil {
t.Errorf("cannot read file: %w", err)
}
var workflow gitHubActionWorkflowConfig
err = yaml.Unmarshal(content, &workflow)
if err != nil {
t.Errorf("cannot unmarshal file: %w", err)
}
actualShells := make([]string, 0)
for _, job := range workflow.Jobs {
job := job
for _, step := range job.Steps {
step := step
shell, err := getShellForStep(&step, &job)
if err != nil {
t.Errorf("error getting shell: %w", err)
}
actualShells = append(actualShells, shell)
}
}
if !cmp.Equal(tt.expectedShells, actualShells) {
t.Errorf("%v: Got (%v) expected (%v)", tt.name, actualShells, tt.expectedShells)
}
})
}
}
20 changes: 20 additions & 0 deletions checks/testdata/github-workflow-shells-all-windows-bash.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright 2021 Security 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.

jobs:
Job1:
runs-on: [windows-2019, windows-latest]
steps:
- run: echo "hello, github"
shell: bash
22 changes: 22 additions & 0 deletions checks/testdata/github-workflow-shells-all-windows-matrix.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Copyright 2021 Security 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.

jobs:
Job1:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [windows-2019, windows-latest]
steps:
- run: echo "hello, github"
19 changes: 19 additions & 0 deletions checks/testdata/github-workflow-shells-all-windows.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright 2021 Security 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.

jobs:
Job1:
runs-on: [windows-2019, windows-latest]
steps:
- run: echo "hello, github"

0 comments on commit 1c7ba79

Please sign in to comment.