Skip to content

Commit

Permalink
feat: support verifying if the commit hash of a semver comment is equ…
Browse files Browse the repository at this point in the history
…ivalent to a commit SHA of action version (#439)

* feat: support verifying if the commit hash of a semver comment is equivalent to a commit SHA of action version

* fix: improve the error log and add document

* docs: update document
  • Loading branch information
suzuki-shunsuke authored May 25, 2024
1 parent 26a2d0e commit 134cde2
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 32 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ index 84bd67a..5d92e44 100644
permissions:
```

[pinact also supports verifying version annotations](docs/codes/001.md).

## Motivation

It is a good manner to pin GitHub Actions versions by commit hash.
Expand Down Expand Up @@ -135,6 +137,10 @@ $ pinact init '.github/pinact.yaml'

About the configuration, please see [Configuration](#Configuration).

## Verify version annotations

Please see [the document](docs/codes/001.md).

## GitHub Actions

https://github.com/suzuki-shunsuke/pinact-action
Expand Down
33 changes: 33 additions & 0 deletions docs/codes/001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Verify version annotations

pinact >= v0.1.3

Please see the following code.

```yaml
- uses: actions/checkout@ee0669bd1cc54295c223e0bb666b733df41de1c5 # v3.5.1
```
You would assume the version of the action is v3.5.1 because the version annotation is "v3.5.1".
But the actual version is v2.7.0 because "ee0669bd1cc54295c223e0bb666b733df41de1c5" is the commit hash of v2.7.0.
Please check releases.
- https://github.com/actions/checkout/releases/tag/v3.5.1
- https://github.com/actions/checkout/releases/tag/v2.7.0
This indicates version annotations aren't necessarily correct.
Especially, attackers can specify a full commit SHA including a malicious code while setting a safe tag to the version annotation.
If a pull request includes changes of GitHub Actions, you should verify version annotations.
pinact v0.1.3 or newer can verify version annotations using `pinact run`'s `--verify` option.
This verification works only if the version annotation is semver and the version is full commit hash like the above example.
This option gets a full commit hash from a version annotation by GitHub API and compares it with the version.

e.g.

```console
$ pinact run --verify testdata/bar.yaml
ERRO[0000] parse a line action=actions/checkout action_version=ee0669bd1cc54295c223e0bb666b733df41de1c5 commit_hash_of_version_annotation=83b7061638ee4956cf7545a6f7efe594e5ad0247 error="verify the version annotation: action_version must be equal to commit_hash_of_version_annotation" help_docs="https://github.com/suzuki-shunsuke/pinact/blob/main/docs/codes/001.md" pinact_version= program=pinact version_annotation=v3.5.1 workflow_file=testdata/bar.yaml
```

Note that `--verify` option calls GitHub API to verify version annotations, which may cause API rate limiting.
8 changes: 8 additions & 0 deletions pkg/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ e.g.
$ pinact run .github/actions/foo/action.yaml .github/actions/bar/action.yaml
`,
Action: r.runAction,
Flags: []cli.Flag{
&cli.BoolFlag{
Name: "verify",
Aliases: []string{"v"},
Usage: "verify if pairs of commit SHA and version are correct",
},
},
}
}

Expand All @@ -38,6 +45,7 @@ func (r *Runner) runAction(c *cli.Context) error {
WorkflowFilePaths: c.Args().Slice(),
ConfigFilePath: c.String("config"),
PWD: pwd,
IsVerify: c.Bool("verify"),
}
return ctrl.Run(c.Context, r.LogE, param) //nolint:wrapcheck
}
1 change: 1 addition & 0 deletions pkg/controller/run/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
type Config struct {
Files []*File
IgnoreActions []*IgnoreAction `yaml:"ignore_actions"`
IsVerify bool `yaml:"-"`
}

type File struct {
Expand Down
130 changes: 98 additions & 32 deletions pkg/controller/run/parse_line.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package run

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
Expand All @@ -11,7 +12,12 @@ import (
"github.com/suzuki-shunsuke/pinact/pkg/github"
)

var usesPattern = regexp.MustCompile(`^ +(?:- )?uses: +(.*)@([^ ]+)(?: +# +(?:tag=)?(v\d+[^ ]*))?`)
var (
usesPattern = regexp.MustCompile(`^ +(?:- )?uses: +(.*)@([^ ]+)(?: +# +(?:tag=)?(v?\d+[^ ]*))?`)
fullCommitSHAPattern = regexp.MustCompile(`\b[0-9a-f]{40}\b`)
semverPattern = regexp.MustCompile(`^v?\d+\.\d+\.\d+[^ ]*$`)
shortTagPattern = regexp.MustCompile(`^v\d+$`)
)

type Action struct {
Name string
Expand All @@ -21,6 +27,32 @@ type Action struct {
RepoName string
}

type VersionType int

const (
Semver VersionType = iota
Shortsemver
FullCommitSHA
Empty
Other
)

func getVersionType(v string) VersionType {
if v == "" {
return Empty
}
if fullCommitSHAPattern.MatchString(v) {
return FullCommitSHA
}
if semverPattern.MatchString(v) {
return Semver
}
if shortTagPattern.MatchString(v) {
return Shortsemver
}
return Other
}

func (c *Controller) parseLine(ctx context.Context, logE *logrus.Entry, line string, cfg *Config) (string, error) { //nolint:cyclop,funlen
matches := usesPattern.FindStringSubmatch(line)
if matches == nil {
Expand All @@ -29,9 +61,9 @@ func (c *Controller) parseLine(ctx context.Context, logE *logrus.Entry, line str
return line, nil
}
action := &Action{
Name: matches[1],
Name: matches[1], // local action is excluded by the regular expression because local action doesn't have version @
Version: matches[2], // full commit hash, main, v3, v3.0.0
Tag: matches[3], // empty, v1, v3.0.0, hoge
Tag: matches[3], // empty, v1, v3.0.0
}

for _, ignoreAction := range cfg.IgnoreActions {
Expand All @@ -44,11 +76,19 @@ func (c *Controller) parseLine(ctx context.Context, logE *logrus.Entry, line str
}
}

if f := c.parseAction(action); !f {
if f := c.parseActionName(action); !f {
logE.WithField("line", line).Debug("ignore line")
return line, nil
}
if action.Tag == "" { //nolint:nestif

switch getVersionType(action.Tag) {
case Empty:
typ := getVersionType(action.Version)
switch typ {
case Shortsemver, Semver:
default:
return line, nil
}
// @xxx
// Get commit hash from tag
// https://docs.github.com/en/rest/git/refs?apiVersion=2022-11-28#get-a-reference
Expand All @@ -59,7 +99,7 @@ func (c *Controller) parseLine(ctx context.Context, logE *logrus.Entry, line str
return line, nil
}
longVersion := action.Version
if shortTagPattern.MatchString(action.Version) {
if typ == Shortsemver {
v, err := c.getLongVersionFromSHA(ctx, action, sha)
if err != nil {
return "", err
Expand All @@ -70,23 +110,39 @@ func (c *Controller) parseLine(ctx context.Context, logE *logrus.Entry, line str
}
// @yyy # longVersion
return c.patchLine(line, action, sha, longVersion), nil
}
// @xxx # v3
// list releases
// extract releases by commit hash
if !shortTagPattern.MatchString(action.Tag) {
logE.WithField("action_version", action.Version).Debug("ignore the line because the tag is not short")
case Semver:
// verify commit hash
if !cfg.IsVerify {
return line, nil
}
// @xxx # v3.0.0
// @<full commit hash> # v3.0.0
if FullCommitSHA != getVersionType(action.Version) {
return line, nil
}
if err := c.verify(ctx, action); err != nil {
return "", fmt.Errorf("verify the version annotation: %w", err)
}
return line, nil
}
longVersion, err := c.getLongVersionFromSHA(ctx, action, action.Version)
if err != nil {
return "", err
}
if longVersion == "" {
logE.Debug("failed to get a long tag")
case Shortsemver:
// @xxx # v3
// @<full commit hash> # v3
if FullCommitSHA != getVersionType(action.Version) {
return line, nil
}
// replace Shortsemer to Semver
longVersion, err := c.getLongVersionFromSHA(ctx, action, action.Version)
if err != nil {
return "", err
}
if longVersion == "" {
logE.Debug("failed to get a long tag")
return line, nil
}
return c.patchLine(line, action, action.Version, longVersion), nil
default:
return line, nil
}
return c.patchLine(line, action, action.Version, longVersion), nil
}

func (c *Controller) patchLine(line string, action *Action, version, tag string) string {
Expand All @@ -105,7 +161,7 @@ func (c *Controller) getLongVersionFromSHA(ctx context.Context, action *Action,
}
// Get long tag from commit hash
for range 10 {
tags, _, err := c.repositoriesService.ListTags(ctx, action.RepoOwner, action.RepoName, opts)
tags, resp, err := c.repositoriesService.ListTags(ctx, action.RepoOwner, action.RepoName, opts)
if err != nil {
return "", fmt.Errorf("list tags: %w", err)
}
Expand All @@ -127,30 +183,40 @@ func (c *Controller) getLongVersionFromSHA(ctx context.Context, action *Action,
return tagName, nil
}
}
if len(tags) < opts.PerPage {
if resp.NextPage == 0 {
return "", nil
}
opts.Page++
opts.Page = resp.NextPage
}
return "", nil
}

var shortTagPattern = regexp.MustCompile(`^v\d+$`)

// parseAction returns true if the action is a target.
// parseActionName returns true if the action is a target.
// Otherwise, it returns false.
func (c *Controller) parseAction(action *Action) bool {
func (c *Controller) parseActionName(action *Action) bool {
a := strings.Split(action.Name, "/")
if len(a) == 1 {
// If it fails to extract the repository owner and name, ignore the action.
return false
}
action.RepoOwner = a[0]
action.RepoName = a[1]
if action.Tag != "" && !shortTagPattern.MatchString(action.Tag) {
// Ignore if the tag is not a short tag.
// e.g. uses: actions/checkout@xxx # v2.0.0
return false
}
return true
}

func (c *Controller) verify(ctx context.Context, action *Action) error {
sha, _, err := c.repositoriesService.GetCommitSHA1(ctx, action.RepoOwner, action.RepoName, action.Tag, "")
if err != nil {
return fmt.Errorf("get a commit hash: %w", err)
}
if action.Version == sha {
return nil
}
return logerr.WithFields(errors.New("action_version must be equal to commit_hash_of_version_annotation"), logrus.Fields{ //nolint:wrapcheck
"action": action.Name,
"action_version": action.Version,
"version_annotation": action.Tag,
"commit_hash_of_version_annotation": sha,
"help_docs": "https://github.com/suzuki-shunsuke/pinact/blob/main/docs/codes/001.md",
})
}
3 changes: 3 additions & 0 deletions pkg/controller/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,20 @@ type ParamRun struct {
WorkflowFilePaths []string
ConfigFilePath string
PWD string
IsVerify bool
}

func (c *Controller) Run(ctx context.Context, logE *logrus.Entry, param *ParamRun) error {
cfg := &Config{}
if err := c.readConfig(param.ConfigFilePath, cfg); err != nil {
return err
}
cfg.IsVerify = param.IsVerify
workflowFilePaths, err := c.searchFiles(logE, param.WorkflowFilePaths, cfg, param.PWD)
if err != nil {
return fmt.Errorf("search target files: %w", err)
}

for _, workflowFilePath := range workflowFilePaths {
logE := logE.WithField("workflow_file", workflowFilePath)
if err := c.runWorkflow(ctx, logE, workflowFilePath, cfg); err != nil {
Expand Down
15 changes: 15 additions & 0 deletions testdata/bar.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
name: bar
on: workflow_call
jobs:
integration-test:
runs-on: ubuntu-latest
permissions: {}
steps:
# The version annotation is "v3.5.1", so you would think the version of the action is v3.5.1.
# But the actual version is v2.7.0 because "ee0669bd1cc54295c223e0bb666b733df41de1c5" is the commit hash of v2.7.0.
# https://github.com/actions/checkout/releases/tag/v3.5.1
# https://github.com/actions/checkout/releases/tag/v2.7.0
# This means version annotations aren't necessarily correct.
# pinact run's --verify option verifies version annoations.
- uses: actions/checkout@ee0669bd1cc54295c223e0bb666b733df41de1c5 # v3.5.1

0 comments on commit 134cde2

Please sign in to comment.