Skip to content

Commit

Permalink
feat: add a policy deny_inherit_secrets (#214)
Browse files Browse the repository at this point in the history
  • Loading branch information
suzuki-shunsuke authored Nov 16, 2023
1 parent 4d24dfe commit 64672f0
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 1 deletion.
26 changes: 25 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# ghalint

GitHub Actions linter
GitHub Actions linter for security best practices.

[Blog post](https://dev.to/suzukishunsuke/minimize-the-scope-of-secrets-and-permissions-in-github-actions-444b)

Expand All @@ -15,6 +15,8 @@ GitHub Actions linter
- Why: For least privilege
- `deny_write_all_permission`: `write-all` permission should not be used
- Why: For least privilege
- `deny_inherit_secrets`: `secrets: inherit` should not be used
- Why: Secrets should be exposed to only required jobs
- `workflow_secrets`: Workflow should not set secrets to environment variables
- How to fix: set secrets to jobs
- Why: To limit the scope of secrets
Expand Down Expand Up @@ -111,6 +113,28 @@ jobs:
Same with `deny_read_all_permission`.

## deny_inherit_secrets

:x:

```yaml
jobs:
release:
uses: suzuki-shunsuke/go-release-workflow/.github/workflows/release.yaml@v0.4.4
secrets: inherit # `inherit` should not be used
```

:o:

```yaml
jobs:
release:
uses: suzuki-shunsuke/go-release-workflow/.github/workflows/release.yaml@v0.4.4
secrets: # Only required secrets should be passed
gh_app_id: ${{ secrets.APP_ID }}
gh_app_private_key: ${{ secrets.APP_PRIVATE_KEY }}
```
### workflow_secrets
:x:
Expand Down
29 changes: 29 additions & 0 deletions pkg/cli/deny_inherit_secrets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package cli

import (
"context"
"errors"

"github.com/sirupsen/logrus"
)

type DenyInheritSecretsPolicy struct{}

func (policy *DenyInheritSecretsPolicy) Name() string {
return "deny_inherit_secrets"
}

func (policy *DenyInheritSecretsPolicy) Apply(ctx context.Context, logE *logrus.Entry, cfg *Config, wf *Workflow) error {
failed := false
for jobName, job := range wf.Jobs {
logE := logE.WithField("job_name", jobName)
if job.Secrets.Inherit() {
failed = true
logE.Error("`secrets: inherit` should not be used. Only required secrets should be passed explicitly")
}
}
if failed {
return errors.New("workflow violates policies")
}
return nil
}
68 changes: 68 additions & 0 deletions pkg/cli/job_secrets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package cli

import (
"errors"

"github.com/sirupsen/logrus"
"github.com/suzuki-shunsuke/logrus-error/logerr"
)

type JobSecrets struct {
m map[string]string
inherit bool
}

func (js *JobSecrets) Inherit() bool {
return js != nil && js.inherit
}

func (js *JobSecrets) UnmarshalYAML(unmarshal func(interface{}) error) error {
var val interface{}
if err := unmarshal(&val); err != nil {
return err
}
return convJobSecrets(val, js)
}

func convJobSecrets(src interface{}, dest *JobSecrets) error { //nolint:cyclop
switch p := src.(type) {
case string:
switch p {
case "inherit":
dest.inherit = true
return nil
default:
return logerr.WithFields(errors.New("job secrets must be a map or `inherit`"), logrus.Fields{ //nolint:wrapcheck
"secrets": p,
})
}
case map[interface{}]interface{}:
m := make(map[string]string, len(p))
for k, v := range p {
ks, ok := k.(string)
if !ok {
return errors.New("secrets key must be string")
}
vs, ok := v.(string)
if !ok {
return errors.New("secrets value must be string")
}
m[ks] = vs
}
dest.m = m
return nil
case map[string]interface{}:
m := make(map[string]string, len(p))
for k, v := range p {
vs, ok := v.(string)
if !ok {
return errors.New("secrets value must be string")
}
m[k] = vs
}
dest.m = m
return nil
default:
return errors.New("secrets must be map[string]string or 'inherit'")
}
}
2 changes: 2 additions & 0 deletions pkg/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (runner *Runner) Run(ctx *cli.Context) error {
NewJobSecretsPolicy(),
&DenyReadAllPermissionPolicy{},
&DenyWriteAllPermissionPolicy{},
&DenyInheritSecretsPolicy{},
}
failed := false
for _, filePath := range filePaths {
Expand Down Expand Up @@ -90,6 +91,7 @@ type Job struct {
Permissions *Permissions
Env map[string]string
Steps []interface{}
Secrets *JobSecrets
}

type Permissions struct {
Expand Down

0 comments on commit 64672f0

Please sign in to comment.