Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Ignore files under .github/workflows/<> in Packaging check #1591

Merged
merged 10 commits into from
Feb 4, 2022
Merged

🐛 Ignore files under .github/workflows/<> in Packaging check #1591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2b617c0
checks/packaging.go: ignore workflows/<>/ files
laurentsimon Feb 2, 2022
62ceeb2
share function
laurentsimon Feb 2, 2022
57e9b76
move tests
laurentsimon Feb 2, 2022
ec5fccb
linter
laurentsimon Feb 3, 2022
39c2e04
tests
laurentsimon Feb 3, 2022
518f4c3
tests
laurentsimon Feb 3, 2022
56430d9
utests
laurentsimon Feb 4, 2022
d147660
tests
laurentsimon Feb 4, 2022
91899da
fix
laurentsimon Feb 4, 2022
e28c173
fix
laurentsimon Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions checks/dangerous_workflow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package checks
import (
"fmt"
"io/ioutil"
"strings"
"testing"

"github.com/ossf/scorecard/v4/checker"
Expand Down Expand Up @@ -44,7 +45,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run untrusted code checkout test",
filename: "./testdata/github-workflow-dangerous-pattern-untrusted-checkout.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-checkout.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultScore,
Expand All @@ -55,7 +56,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run trusted code checkout test",
filename: "./testdata/github-workflow-dangerous-pattern-trusted-checkout.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-trusted-checkout.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore,
Expand All @@ -66,7 +67,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run default code checkout test",
filename: "./testdata/github-workflow-dangerous-pattern-default-checkout.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-default-checkout.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore,
Expand All @@ -77,7 +78,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run safe trigger with code checkout test",
filename: "./testdata/github-workflow-dangerous-pattern-safe-trigger.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-safe-trigger.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore,
Expand All @@ -88,7 +89,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run script injection",
filename: "./testdata/github-workflow-dangerous-pattern-untrusted-script-injection.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-script-injection.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultScore,
Expand All @@ -99,7 +100,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run safe script injection",
filename: "./testdata/github-workflow-dangerous-pattern-trusted-script-injection.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-trusted-script-injection.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore,
Expand All @@ -110,7 +111,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run multiple script injection",
filename: "./testdata/github-workflow-dangerous-pattern-untrusted-multiple-script-injection.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-multiple-script-injection.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
Expand All @@ -121,7 +122,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run inline script injection",
filename: "./testdata/github-workflow-dangerous-pattern-untrusted-inline-script-injection.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-inline-script-injection.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
Expand All @@ -132,7 +133,7 @@ func TestGithubDangerousWorkflow(t *testing.T) {
},
{
name: "run wildcard script injection",
filename: "./testdata/github-workflow-dangerous-pattern-untrusted-script-injection-wildcard.yml",
filename: "./testdata/.github/workflows/github-workflow-dangerous-pattern-untrusted-script-injection-wildcard.yml",
expected: scut.TestReturn{
Error: nil,
Score: checker.MinResultConfidence,
Expand All @@ -157,7 +158,9 @@ func TestGithubDangerousWorkflow(t *testing.T) {
}
}
dl := scut.TestDetailLogger{}
r := testValidateGitHubActionDangerousWorkflow(tt.filename, content, &dl)
p := strings.Replace(tt.filename, "./testdata/", "", 1)

r := testValidateGitHubActionDangerousWorkflow(p, content, &dl)
if !scut.ValidateTestReturn(t, tt.name, &tt.expected, &r, &dl) {
t.Fail()
}
Expand Down
9 changes: 8 additions & 1 deletion checks/fileparser/github_workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package fileparser
import (
"fmt"
"path"
"path/filepath"
"regexp"
"strings"

Expand Down Expand Up @@ -292,12 +293,18 @@ func IsWorkflowFile(pathfn string) bool {
// "Workflow files use YAML syntax, and must have either a .yml or .yaml file extension."
switch path.Ext(pathfn) {
case ".yml", ".yaml":
return true
return filepath.Dir(strings.ToLower(pathfn)) == ".github/workflows"
default:
return false
}
}

// IsGithubWorkflowFileCb determines if a file is a workflow
// as a callback to use for repo client's ListFiles() API.
func IsGithubWorkflowFileCb(pathfn string) (bool, error) {
return IsWorkflowFile(pathfn), nil
}

// IsGitHubOwnedAction checks if this is a github specific action.
func IsGitHubOwnedAction(actionName string) bool {
a := strings.HasPrefix(actionName, "actions/")
Expand Down
46 changes: 24 additions & 22 deletions checks/fileparser/github_workflow_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package fileparser

import (
stdos "os"
"strings"
"testing"

"github.com/rhysd/actionlint"
Expand All @@ -41,77 +42,77 @@ func TestGitHubWorkflowShell(t *testing.T) {
}{
{
name: "all windows, shell specified in step",
filename: "../testdata/github-workflow-shells-all-windows-bash.yaml",
filename: "../testdata/.github/workflows/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",
filename: "../testdata/.github/workflows/github-workflow-shells-all-windows-matrix.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "all windows, OSes listed in matrix.include",
filename: "../testdata/github-workflow-shells-all-windows-matrix-include.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-all-windows-matrix-include.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "all windows, empty matrix.include",
filename: "../testdata/github-workflow-shells-all-windows-matrix-include-empty.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-all-windows-matrix-include-empty.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "all windows",
filename: "../testdata/github-workflow-shells-all-windows.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-all-windows.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "macOS defaults to bash",
filename: "../testdata/github-workflow-shells-default-macos.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-default-macos.yaml",
expectedShells: []string{"bash"},
},
{
name: "ubuntu defaults to bash",
filename: "../testdata/github-workflow-shells-default-ubuntu.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-default-ubuntu.yaml",
expectedShells: []string{"bash"},
},
{
name: "windows defaults to pwsh",
filename: "../testdata/github-workflow-shells-default-windows.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-default-windows.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "windows specified in 'if'",
filename: "../testdata/github-workflow-shells-runner-windows-ubuntu.yaml",
filename: "../testdata/.github/workflows/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",
filename: "../testdata/.github/workflows/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",
filename: "../testdata/.github/workflows/github-workflow-shells-specified-job-windows.yaml",
expectedShells: []string{"bash"},
},
{
name: "shell specified in job",
filename: "../testdata/github-workflow-shells-specified-job.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-specified-job.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "shell specified in step",
filename: "../testdata/github-workflow-shells-speficied-step.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-speficied-step.yaml",
expectedShells: []string{"pwsh"},
},
{
name: "different shells in each step",
filename: "../testdata/github-workflow-shells-two-shells.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-two-shells.yaml",
expectedShells: []string{"bash", "pwsh"},
},
{
name: "windows step, bash specified",
filename: "../testdata/github-workflow-shells-windows-bash.yaml",
filename: "../testdata/.github/workflows/github-workflow-shells-windows-bash.yaml",
expectedShells: []string{"bash", "bash"},
},
}
Expand Down Expand Up @@ -160,42 +161,42 @@ func TestIsWorkflowFile(t *testing.T) {
{
name: "yaml",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.yaml",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.yaml",
},
want: true,
},
{
name: "yml",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.yml",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.yml",
},
want: true,
},
{
name: "json",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.json",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.json",
},
want: false,
},
{
name: "txt",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.txt",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.txt",
},
want: false,
},
{
name: "md",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.md",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.md",
},
want: false,
},
{
name: "unknown",
args: args{
pathfn: "../testdata/github-workflow-shells-all-windows.unknown",
pathfn: "./testdata/.github/workflows/github-workflow-shells-all-windows.unknown",
},
want: false,
},
Expand All @@ -205,7 +206,8 @@ func TestIsWorkflowFile(t *testing.T) {
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()
if got := IsWorkflowFile(tt.args.pathfn); got != tt.want {
p := strings.Replace(tt.args.pathfn, "./testdata/", "", 1)
if got := IsWorkflowFile(p); got != tt.want {
t.Errorf("IsWorkflowFile() = %v, want %v for tests %v", got, tt.want, tt.name)
}
})
Expand Down
7 changes: 1 addition & 6 deletions checks/packaging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package checks
import (
"fmt"
"path/filepath"
"strings"

"github.com/rhysd/actionlint"

Expand All @@ -37,13 +36,9 @@ func init() {
}
}

func isGithubWorkflowFile(filename string) (bool, error) {
return strings.HasPrefix(strings.ToLower(filename), ".github/workflows"), nil
}

// Packaging runs Packaging check.
func Packaging(c *checker.CheckRequest) checker.CheckResult {
matchedFiles, err := c.RepoClient.ListFiles(isGithubWorkflowFile)
matchedFiles, err := c.RepoClient.ListFiles(fileparser.IsGithubWorkflowFileCb)
if err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListFiles: %v", err))
return checker.CreateRuntimeErrorResult(CheckPackaging, e)
Expand Down
27 changes: 15 additions & 12 deletions checks/packaging_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package checks
import (
"fmt"
"os"
"strings"
"testing"

"github.com/rhysd/actionlint"
Expand All @@ -34,57 +35,57 @@ func TestIsPackagingWorkflow(t *testing.T) {
}{
{
name: "npmjs.org publish",
filename: "./testdata/github-workflow-packaging-npm.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-npm.yaml",
expected: true,
},
{
name: "npm github publish",
filename: "./testdata/github-workflow-packaging-npm-github.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-npm-github.yaml",
expected: false, // Should this be false?
},
{
name: "maven publish",
filename: "./testdata/github-workflow-packaging-maven.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-maven.yaml",
expected: true,
},
{
name: "gradle publish",
filename: "./testdata/github-workflow-packaging-gradle.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-gradle.yaml",
expected: true,
},
{
name: "gem publish",
filename: "./testdata/github-workflow-packaging-gem.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-gem.yaml",
expected: true,
},
{
name: "nuget publish",
filename: "./testdata/github-workflow-packaging-nuget.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-nuget.yaml",
expected: true,
},
{
name: "docker action publish",
filename: "./testdata/github-workflow-packaging-docker-action.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-docker-action.yaml",
expected: true,
},
{
name: "docker push publish",
filename: "./testdata/github-workflow-packaging-docker-push.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-docker-push.yaml",
expected: true,
},
{
name: "pypi publish",
filename: "./testdata/github-workflow-packaging-pypi.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-pypi.yaml",
expected: true,
},
{
name: "go publish",
filename: "./testdata/github-workflow-packaging-go.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-go.yaml",
expected: true,
},
{
name: "cargo publish",
filename: "./testdata/github-workflow-packaging-cargo.yaml",
filename: "./testdata/.github/workflows/github-workflow-packaging-cargo.yaml",
expected: true,
},
}
Expand All @@ -101,7 +102,9 @@ func TestIsPackagingWorkflow(t *testing.T) {
panic(fmt.Errorf("cannot parse file: %w", err))
}
dl := scut.TestDetailLogger{}
result := isPackagingWorkflow(workflow, tt.filename, &dl)
p := strings.Replace(tt.filename, "./testdata/", "", 1)

result := isPackagingWorkflow(workflow, p, &dl)
if result != tt.expected {
t.Errorf("isPackagingWorkflow() = %v, expected %v", result, tt.expected)
}
Expand Down
Loading