From c911508c470885e7f2d0dfc7f625e8f61016f7b0 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Mon, 30 Jan 2023 22:59:11 +0900 Subject: [PATCH 1/2] feat: support configuration file and a policy `job_secrets` --- README.md | 54 +++++++++++++++++++++++++ pkg/cli/config.go | 11 +++++ pkg/cli/job_permissions_policy.go | 2 +- pkg/cli/job_secrets_policy.go | 64 ++++++++++++++++++++++++++++++ pkg/cli/run.go | 60 +++++++++++++++++++++++++++- pkg/cli/workflow_secrets_policy.go | 2 +- 6 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 pkg/cli/config.go create mode 100644 pkg/cli/job_secrets_policy.go diff --git a/README.md b/README.md index d1b4935..47938b7 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,11 @@ GitHub Actions linter - Why: To limit the scope of secrets - Exceptions - workflow has only one job +- `job_secrets`: Job should not set secrets to environment variables + - How to fix: set secrets to steps + - Why: To limit the scope of secrets + - Exceptions + - job has only one step ### job_permissions @@ -112,6 +117,38 @@ jobs: - run: echo bar ``` +### job_secrets + +:x: + +```yaml +jobs: + foo: + runs-on: ubuntu-latest + permissions: + issues: write + env: + GITHUB_TOKEN: ${{github.token}} # secret is set in job + steps: + - run: echo foo + - run: gh label create bug +``` + +:o: + +```yaml +jobs: + foo: + runs-on: ubuntu-latest + permissions: + issues: write + steps: + - run: echo foo + - run: gh label create bug + env: + GITHUB_TOKEN: ${{github.token}} # secret is set in step +``` + ## How to install - [Download a pre-built binary from GitHub Releases](https://github.com/suzuki-shunsuke/ghalint/releases) and locate an executable binary `ghalint` in `PATH` @@ -131,6 +168,23 @@ ERRO[0000] github.token should not be set to workflow's env env_name=GITHUB_TOK ERRO[0000] secret should not be set to workflow's env env_name=DATADOG_API_KEY policy_name=workflow_secrets program=ghalint version= workflow_file_path=.github/workflows/test.yaml ``` +## Configuration file + +Configuration file path: `^\.?ghalint\.ya?ml$` + +You can exclude the policy `job_secrets`. + +e.g. + +```yaml +excludes: + - policy_name: job_secrets + workflow_file_path: .github/workflows/actionlint.yaml + job_name: actionlint +``` + +* policy_name: Only `job_secrets` is supported + ## How does it works? ghalint reads GitHub Actions Workflows `^\.github/workflows/.*\.ya?ml$` and validates them. diff --git a/pkg/cli/config.go b/pkg/cli/config.go new file mode 100644 index 0000000..f0d5cf4 --- /dev/null +++ b/pkg/cli/config.go @@ -0,0 +1,11 @@ +package cli + +type Config struct { + Excludes []*Exclude +} + +type Exclude struct { + PolicyName string `yaml:"policy_name"` + WorkflowFilePath string `yaml:"workflow_file_path"` + JobName string `yaml:"job_name"` +} diff --git a/pkg/cli/job_permissions_policy.go b/pkg/cli/job_permissions_policy.go index c1c6025..bcf2e7b 100644 --- a/pkg/cli/job_permissions_policy.go +++ b/pkg/cli/job_permissions_policy.go @@ -13,7 +13,7 @@ func (policy *JobPermissionsPolicy) Name() string { return "job_permissions" } -func (policy *JobPermissionsPolicy) Apply(ctx context.Context, logE *logrus.Entry, wf *Workflow) error { +func (policy *JobPermissionsPolicy) Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error { failed := false if wf.Permissions != nil && len(wf.Permissions) == 0 { return nil diff --git a/pkg/cli/job_secrets_policy.go b/pkg/cli/job_secrets_policy.go new file mode 100644 index 0000000..7d16222 --- /dev/null +++ b/pkg/cli/job_secrets_policy.go @@ -0,0 +1,64 @@ +package cli + +import ( + "context" + "errors" + "regexp" + + "github.com/sirupsen/logrus" +) + +type JobSecretsPolicy struct { + secretPattern *regexp.Regexp + githubTokenPattern *regexp.Regexp +} + +func NewJobSecretsPolicy() *JobSecretsPolicy { + return &JobSecretsPolicy{ + secretPattern: regexp.MustCompile(`\${{ *secrets\.[^ ]+ *}}`), + githubTokenPattern: regexp.MustCompile(`\${{ *github\.token+ *}}`), + } +} + +func (policy *JobSecretsPolicy) Name() string { + return "job_secrets" +} + +func checkExcludes(policyName string, wf *Workflow, jobName string, cfg *Config) bool { + for _, exclude := range cfg.Excludes { + if exclude.PolicyName == policyName && wf.FilePath == exclude.WorkflowFilePath && jobName == exclude.JobName { + return true + } + } + return false +} + +func (policy *JobSecretsPolicy) Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error { + if len(wf.Jobs) < 2 { //nolint:gomnd + return nil + } + failed := false + for jobName, job := range wf.Jobs { + logE := logE.WithField("job_name", jobName) + if checkExcludes(policy.Name(), wf, jobName, cfg) { + continue + } + if len(job.Steps) < 2 { //nolint:gomnd + continue + } + for envName, envValue := range job.Env { + if policy.secretPattern.MatchString(envValue) { + failed = true + logE.WithField("env_name", envName).Error("secret should not be set to job's env") + } + if policy.githubTokenPattern.MatchString(envValue) { + failed = true + logE.WithField("env_name", envName).Error("github.token should not be set to job's env") + } + } + } + if failed { + return errors.New("workflow violates policies") + } + return nil +} diff --git a/pkg/cli/run.go b/pkg/cli/run.go index 7bbcd73..69ef684 100644 --- a/pkg/cli/run.go +++ b/pkg/cli/run.go @@ -3,15 +3,68 @@ package cli import ( "context" "errors" + "fmt" + "os" "github.com/sirupsen/logrus" "github.com/suzuki-shunsuke/ghalint/pkg/log" "github.com/suzuki-shunsuke/logrus-error/logerr" "github.com/urfave/cli/v2" + "gopkg.in/yaml.v3" ) +func findConfig() string { + for _, filePath := range []string{"ghalint.yaml", ".ghalint.yaml", "ghalint.yml", ".ghalint.yml"} { + if _, err := os.Stat(filePath); err == nil { + return filePath + } + } + return "" +} + +func readConfig(cfg *Config, filePath string) error { + f, err := os.Open(filePath) + if err != nil { + return fmt.Errorf("open a configuration file: %w", err) + } + defer f.Close() + if err := yaml.NewDecoder(f).Decode(cfg); err != nil { + return fmt.Errorf("parse configuration file as YAML: %w", err) + } + return nil +} + +func validateConfig(cfg *Config) error { + for _, exclude := range cfg.Excludes { + if exclude.PolicyName == "" { + return errors.New(`policy_name is required`) + } + if exclude.PolicyName != "job_secrets" { + return errors.New(`only the policy "job_secrets" can be excluded`) + } + if exclude.WorkflowFilePath == "" { + return errors.New(`workflow_file_path is required`) + } + if exclude.JobName == "" { + return errors.New(`jobName is required`) + } + } + return nil +} + func (runner *Runner) Run(ctx *cli.Context) error { logE := log.New(runner.flags.Version) + cfg := &Config{} + if cfgFilePath := findConfig(); cfgFilePath != "" { + if err := readConfig(cfg, cfgFilePath); err != nil { + logE.WithError(err).Error("read a configuration file") + return err + } + } + if err := validateConfig(cfg); err != nil { + logE.WithError(err).Error("validate a configuration file") + return err + } filePaths, err := listWorkflows() if err != nil { logE.Error(err) @@ -20,6 +73,7 @@ func (runner *Runner) Run(ctx *cli.Context) error { policies := []Policy{ &JobPermissionsPolicy{}, NewWorkflowSecretsPolicy(), + NewJobSecretsPolicy(), } failed := false for _, filePath := range filePaths { @@ -35,7 +89,7 @@ func (runner *Runner) Run(ctx *cli.Context) error { for _, policy := range policies { logE := logE.WithField("policy_name", policy.Name()) - if err := policy.Apply(ctx.Context, logE, wf); err != nil { + if err := policy.Apply(ctx.Context, logE, cfg, wf); err != nil { failed = true continue } @@ -49,7 +103,7 @@ func (runner *Runner) Run(ctx *cli.Context) error { type Policy interface { Name() string - Apply(ctx context.Context, logE *logrus.Entry, wf *Workflow) error + Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error } type Workflow struct { @@ -61,4 +115,6 @@ type Workflow struct { type Job struct { Permissions map[string]string + Env map[string]string + Steps []interface{} } diff --git a/pkg/cli/workflow_secrets_policy.go b/pkg/cli/workflow_secrets_policy.go index 9731fed..d7b9543 100644 --- a/pkg/cli/workflow_secrets_policy.go +++ b/pkg/cli/workflow_secrets_policy.go @@ -24,7 +24,7 @@ func (policy *WorkflowSecretsPolicy) Name() string { return "workflow_secrets" } -func (policy *WorkflowSecretsPolicy) Apply(ctx context.Context, logE *logrus.Entry, wf *Workflow) error { +func (policy *WorkflowSecretsPolicy) Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error { if len(wf.Jobs) < 2 { //nolint:gomnd return nil } From 9f48e04db279043e714289d73f0b9bcac215f06d Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Tue, 31 Jan 2023 09:27:53 +0900 Subject: [PATCH 2/2] fix: fix job_secrets policy and add tests --- pkg/cli/job_permissions_policy_test.go | 72 +++++++++++ pkg/cli/job_secrets_policy.go | 3 - pkg/cli/job_secrets_policy_test.go | 158 ++++++++++++++++++++++++ pkg/cli/workflow_secrets_policy_test.go | 95 ++++++++++++++ 4 files changed, 325 insertions(+), 3 deletions(-) create mode 100644 pkg/cli/job_permissions_policy_test.go create mode 100644 pkg/cli/job_secrets_policy_test.go create mode 100644 pkg/cli/workflow_secrets_policy_test.go diff --git a/pkg/cli/job_permissions_policy_test.go b/pkg/cli/job_permissions_policy_test.go new file mode 100644 index 0000000..6403c27 --- /dev/null +++ b/pkg/cli/job_permissions_policy_test.go @@ -0,0 +1,72 @@ +package cli_test + +import ( + "context" + "testing" + + "github.com/sirupsen/logrus" + "github.com/suzuki-shunsuke/ghalint/pkg/cli" +) + +func TestJobPermissionsPolicy_Apply(t *testing.T) { + t.Parallel() + data := []struct { + name string + cfg *cli.Config + wf *cli.Workflow + exp bool + }{ + { + name: "workflow permissions is empty", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + Permissions: map[string]string{}, + Jobs: map[string]*cli.Job{ + "foo": {}, + "bar": {}, + }, + }, + }, + { + name: "workflow has only one job", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + Permissions: map[string]string{ + "contents": "read", + }, + Jobs: map[string]*cli.Job{ + "foo": {}, + }, + }, + }, + { + name: "job should have permissions", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + Jobs: map[string]*cli.Job{ + "foo": {}, + "bar": {}, + }, + }, + exp: false, + }, + } + policy := &cli.JobPermissionsPolicy{} + logE := logrus.NewEntry(logrus.New()) + ctx := context.Background() + for _, d := range data { + d := d + t.Run(d.name, func(t *testing.T) { + t.Parallel() + if err := policy.Apply(ctx, logE, d.cfg, d.wf); err != nil { + if d.exp { + t.Fatal(err) + } + return + } + if d.exp { + t.Fatal("error must be returned") + } + }) + } +} diff --git a/pkg/cli/job_secrets_policy.go b/pkg/cli/job_secrets_policy.go index 7d16222..eb5fd65 100644 --- a/pkg/cli/job_secrets_policy.go +++ b/pkg/cli/job_secrets_policy.go @@ -34,9 +34,6 @@ func checkExcludes(policyName string, wf *Workflow, jobName string, cfg *Config) } func (policy *JobSecretsPolicy) Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error { - if len(wf.Jobs) < 2 { //nolint:gomnd - return nil - } failed := false for jobName, job := range wf.Jobs { logE := logE.WithField("job_name", jobName) diff --git a/pkg/cli/job_secrets_policy_test.go b/pkg/cli/job_secrets_policy_test.go new file mode 100644 index 0000000..dee2c73 --- /dev/null +++ b/pkg/cli/job_secrets_policy_test.go @@ -0,0 +1,158 @@ +package cli_test + +import ( + "context" + "testing" + + "github.com/sirupsen/logrus" + "github.com/suzuki-shunsuke/ghalint/pkg/cli" +) + +func TestJobSecretsPolicy_Apply(t *testing.T) { //nolint:funlen + t.Parallel() + data := []struct { + name string + cfg *cli.Config + wf *cli.Workflow + exp bool + }{ + { + name: "exclude", + cfg: &cli.Config{ + Excludes: []*cli.Exclude{ + { + PolicyName: "job_secrets", + WorkflowFilePath: ".github/workflows/test.yaml", + JobName: "foo", + }, + }, + }, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Jobs: map[string]*cli.Job{ + "foo": { + Env: map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + Steps: []interface{}{ + map[string]interface{}{ + "run": "echo hello", + }, + map[string]interface{}{ + "run": "echo bar", + }, + }, + }, + }, + }, + }, + { + name: "job has only one step", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Jobs: map[string]*cli.Job{ + "foo": { + Env: map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + Steps: []interface{}{ + map[string]interface{}{ + "run": "echo hello", + }, + }, + }, + }, + }, + }, + { + name: "secret should not be set to job's env", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Jobs: map[string]*cli.Job{ + "foo": { + Env: map[string]string{ + "GITHUB_TOKEN": "${{secrets.GITHUB_TOKEN}}", + }, + Steps: []interface{}{ + map[string]interface{}{ + "run": "echo hello", + }, + map[string]interface{}{ + "run": "echo bar", + }, + }, + }, + }, + }, + exp: false, + }, + { + name: "github token should not be set to job's env", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Jobs: map[string]*cli.Job{ + "foo": { + Env: map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + Steps: []interface{}{ + map[string]interface{}{ + "run": "echo hello", + }, + map[string]interface{}{ + "run": "echo bar", + }, + }, + }, + }, + }, + exp: false, + }, + { + name: "pass", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Jobs: map[string]*cli.Job{ + "foo": { + Env: map[string]string{ + "FOO": "foo", + }, + Steps: []interface{}{ + map[string]interface{}{ + "run": "echo hello", + "env": map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + }, + map[string]interface{}{ + "run": "echo bar", + }, + }, + }, + }, + }, + }, + } + policy := cli.NewJobSecretsPolicy() + logE := logrus.NewEntry(logrus.New()) + ctx := context.Background() + for _, d := range data { + d := d + t.Run(d.name, func(t *testing.T) { + t.Parallel() + if err := policy.Apply(ctx, logE, d.cfg, d.wf); err != nil { + if d.exp { + t.Fatal(err) + } + return + } + if d.exp { + t.Fatal("error must be returned") + } + }) + } +} diff --git a/pkg/cli/workflow_secrets_policy_test.go b/pkg/cli/workflow_secrets_policy_test.go new file mode 100644 index 0000000..df00000 --- /dev/null +++ b/pkg/cli/workflow_secrets_policy_test.go @@ -0,0 +1,95 @@ +package cli_test + +import ( + "context" + "testing" + + "github.com/sirupsen/logrus" + "github.com/suzuki-shunsuke/ghalint/pkg/cli" +) + +func TestWorkflowSecretsPolicy_Apply(t *testing.T) { //nolint:funlen + t.Parallel() + data := []struct { + name string + cfg *cli.Config + wf *cli.Workflow + exp bool + }{ + { + name: "workflow has only one job", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Env: map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + Jobs: map[string]*cli.Job{ + "foo": {}, + }, + }, + }, + { + name: "secret should not be set to workflow's env", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Env: map[string]string{ + "GITHUB_TOKEN": "${{secrets.GITHUB_TOKEN}}", + }, + Jobs: map[string]*cli.Job{ + "foo": {}, + "bar": {}, + }, + }, + exp: false, + }, + { + name: "github token should not be set to workflow's env", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Env: map[string]string{ + "GITHUB_TOKEN": "${{github.token}}", + }, + Jobs: map[string]*cli.Job{ + "foo": {}, + "bar": {}, + }, + }, + exp: false, + }, + { + name: "pass", + cfg: &cli.Config{}, + wf: &cli.Workflow{ + FilePath: ".github/workflows/test.yaml", + Env: map[string]string{ + "FOO": "foo", + }, + Jobs: map[string]*cli.Job{ + "foo": {}, + "bar": {}, + }, + }, + }, + } + policy := cli.NewWorkflowSecretsPolicy() + logE := logrus.NewEntry(logrus.New()) + ctx := context.Background() + for _, d := range data { + d := d + t.Run(d.name, func(t *testing.T) { + t.Parallel() + if err := policy.Apply(ctx, logE, d.cfg, d.wf); err != nil { + if d.exp { + t.Fatal(err) + } + return + } + if d.exp { + t.Fatal("error must be returned") + } + }) + } +}