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.

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

Let ghasum update fail if the gha.sum file is corrupt #22

Merged
merged 2 commits into from
Mar 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 13 additions & 6 deletions SPECIFICATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,19 @@ not possible to process shall exit immediately (it means the file may be edited
by another process leading to an inconsistent state).

If the file lock is obtained, the process shall first read it and parse it
partially to extract the sumfile version. If this fails the process shall exit
immediately. Else it shall recompute checksums for all actions used in the
repository (see [Computing Checksums]) using the best available hashing
algorithm. It shall then store them in a sumfile (see [Storing Checksums]) using
the same sumfile version as before and releases the lock. As a consequence, this
adds missing and removes redundant checksums from the sumfile.
completely to extract the sumfile version. If this fails the process shall exit
immediately unless the `-force` flag is used (see details below). Else it shall
recompute checksums for all actions used in the repository (see [Computing
Checksums]) using the best available hashing algorithm. It shall then store them
in a sumfile (see [Storing Checksums]) using the same sumfile version as before
and releases the lock. As a consequence, this adds missing and removes redundant
checksums from the sumfile.

With the `-force` flag the process will ignore errors in the sumfile and fix
those while updating. If the sumfile version can still be determined from
sumfile it will be used, otherwise the latest available version is used instead.
This option is disabled by default to avoid unknowingly fixing syntax errors in
a sumfile, which is an important fact to know about from a security perspective.

This process does not verify any of the checksums currently in the sumfile.

Expand Down
1 change: 1 addition & 0 deletions cmd/ghasum/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const (

const (
flagNameCache = "cache"
flagNameForce = "force"
flagNameNoCache = "no-cache"
)

Expand Down
6 changes: 5 additions & 1 deletion cmd/ghasum/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
func cmdUpdate(argv []string) error {
var (
flags = flag.NewFlagSet(cmdNameUpdate, flag.ContinueOnError)
flagForce = flags.Bool(flagNameForce, false, "")
flagCache = flags.String(flagNameCache, "", "")
flagNoCache = flags.Bool(flagNameNoCache, false, "")
)
Expand Down Expand Up @@ -57,7 +58,7 @@ func cmdUpdate(argv []string) error {
Cache: c,
}

if err := ghasum.Update(&cfg); err != nil {
if err := ghasum.Update(&cfg, *flagForce); err != nil {
return errors.Join(errUnexpected, err)
}

Expand All @@ -79,6 +80,9 @@ The available flags are:
The location of the cache directory. This is where ghasum stores and
looks up repositories it needs.
Defaults to a directory named .ghasum in the user's home directory.
-force
Force updating the gha.sum file, ignoring errors and fixing them in the
process.
-no-cache
Disable the use of the cache. Makes the -cache flag ineffective.`
}
10 changes: 8 additions & 2 deletions internal/ghasum/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func Initialize(cfg *Config) error {

// Update will update the ghasum checksums for the repository specified in the
// given configuration.
func Update(cfg *Config) error {
func Update(cfg *Config, force bool) error {
file, err := open(cfg.Path)
if err != nil {
return err
Expand All @@ -111,7 +111,13 @@ func Update(cfg *Config) error {

version, err := version(raw)
if err != nil {
return err
if !force {
return errors.Join(ErrSumfileRead, err)
}

if errors.Is(err, sumfile.ErrHeaders) || errors.Is(err, sumfile.ErrVersion) {
version = sumfile.VersionLatest
}
}

actions, err := find(cfg)
Expand Down
4 changes: 4 additions & 0 deletions internal/sumfile/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ var (

// ErrSyntax is the error when a checksum file has a syntax error.
ErrSyntax = errors.New("syntax error")

// ErrVersion is the error when the version is invalid or missing from the
// checksum file.
ErrVersion = errors.New("version error")
)
88 changes: 46 additions & 42 deletions internal/sumfile/sumfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,71 +35,74 @@ type Entry struct {
// if there is a syntax error in the checksum file or if the checksum file is
// otherwise corrupted (for example multiple checksum directives for one Entry).
func Decode(stored string) ([]Entry, error) {
_, entries, err := parseFile(stored)
return entries, err
}

// DecodeVersion parses the given checksum file content to extract the version.
func DecodeVersion(stored string) (Version, error) {
headers, _, parseErr := parseFile(stored)
version, _ := extractVersion(headers)
if parseErr != nil {
return version, parseErr
}

return version, nil
}

// Encode encodes the given checksums according to the specification of the
// given version.
func Encode(version Version, checksums []Entry) (string, error) {
var (
encoded string
err error
)

switch version {
case Version1:
encoded, err = encodeV1(checksums)
default:
err = unknownVersion(version)
}

return fmt.Sprintf("version %d\n\n%s", version, encoded), err
}

func parseFile(stored string) (map[string]string, []Entry, error) {
lines := strings.Split(stored, "\n")
headers, err := parseHeaders(lines)
if err != nil {
return nil, err
return nil, nil, err
}

version, err := extractVersion(headers)
if err != nil {
return nil, err
return headers, nil, err
}

if lines[len(lines)-1] != "" {
err = errors.New("missing final newline")
return nil, errors.Join(ErrSyntax, err)
return headers, nil, errors.Join(ErrSyntax, err)
}

content := []string{}
if len(lines) > len(headers)+1 {
content = lines[len(headers)+1 : len(lines)-1]
}

var checksums []Entry
var entries []Entry
switch version {
case Version1:
checksums, err = decodeV1(content)
entries, err = decodeV1(content)
default:
err = unknownVersion(version)
}

if err != nil {
return nil, err
}

return checksums, nil
}

// DecodeVersion parses the given checksum file content to extract the version.
//
// This function may succeed even if the checksum file is corrupted.
func DecodeVersion(stored string) (Version, error) {
lines := strings.Split(stored, "\n")
headers, err := parseHeaders(lines)
if err != nil {
return 0, err
return headers, entries, err
}

return extractVersion(headers)
}

// Encode encodes the given checksums according to the specification of the
// given version.
func Encode(version Version, checksums []Entry) (string, error) {
var (
encoded string
err error
)

switch version {
case Version1:
encoded, err = encodeV1(checksums)
default:
err = unknownVersion(version)
}

return fmt.Sprintf("version %d\n\n%s", version, encoded), err
return headers, entries, nil
}

func parseHeaders(lines []string) (map[string]string, error) {
Expand All @@ -112,7 +115,7 @@ func parseHeaders(lines []string) (map[string]string, error) {
j := strings.IndexRune(line, ' ')
if j == -1 {
err := fmt.Errorf("invalid header on line %d", i)
return nil, errors.Join(ErrSyntax, err)
return nil, errors.Join(ErrHeaders, err)
}

key := line[0:j]
Expand All @@ -127,18 +130,19 @@ func extractVersion(headers map[string]string) (Version, error) {
version, ok := headers["version"]
if !ok {
err := errors.New("version not found")
return 0, errors.Join(ErrSyntax, err)
return 0, errors.Join(ErrVersion, err)
}

rawVersion, err := strconv.Atoi(version)
if err != nil {
err := errors.New("version not a number")
return 0, errors.Join(ErrSyntax, err)
return 0, errors.Join(ErrVersion, err)
}

return Version(rawVersion), nil
}

func unknownVersion(version Version) error {
return fmt.Errorf("unknown version %d", version)
err := fmt.Errorf("unknown version %d", version)
return errors.Join(ErrVersion, err)
}
10 changes: 10 additions & 0 deletions testdata/update/error.txtar
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# Invalid sumfile
! exec ghasum update invalid/
! stdout 'Ok'
stderr 'an unexpected error occurred'
stderr 'could not decode the checksum file'

# Repo without actions
! exec ghasum update no-actions/
! stdout 'Ok'
Expand All @@ -10,6 +16,10 @@ stderr 'ghasum has not yet been initialized'
stderr 'an unexpected error occurred'
stderr 'ghasum has not yet been initialized'

-- invalid/.github/workflows/gha.sum --
version 1

this-action/is-missing@a-checksum
-- no-actions/.keep --
This file exists to create a repo that does not use Github Actions.
-- uninitialized/.github/workflows/workflow.yml --
Expand Down
112 changes: 112 additions & 0 deletions testdata/update/force.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
# Error in entries
exec ghasum update -cache .cache/ -force entries/
stdout 'Ok'
! stderr .
cmp entries/.github/workflows/gha.sum .want/gha.sum

# Error in headers
exec ghasum update -cache .cache/ -force headers/
stdout 'Ok'
! stderr .
cmp headers/.github/workflows/gha.sum .want/gha.sum

# Error in version
exec ghasum update -cache .cache/ -force nan-version/
stdout 'Ok'
! stderr .
cmp nan-version/.github/workflows/gha.sum .want/gha.sum

# Invalid version
exec ghasum update -cache .cache/ -force invalid-version/
stdout 'Ok'
! stderr .
cmp invalid-version/.github/workflows/gha.sum .want/gha.sum

# Missing version
exec ghasum update -cache .cache/ -force no-version/
stdout 'Ok'
! stderr .
cmp no-version/.github/workflows/gha.sum .want/gha.sum

-- entries/.github/workflows/gha.sum --
version 1

this-action/is-missing@a-checksum
-- entries/.github/workflows/workflow.yml --
name: Example workflow
on: [push]

jobs:
example:
name: example
runs-on: ubuntu-22.04
steps:
- name: Checkout repository
uses: actions/[email protected]
-- headers/.github/workflows/gha.sum --
invalid-header

actions/[email protected] GGAV+/JnlPt41B9iINyvcX5z6a4ue+NblmwiDNVORz0=
-- headers/.github/workflows/workflow.yml --
name: Example workflow
on: [push]

jobs:
example:
name: example
runs-on: ubuntu-22.04
steps:
- name: Checkout repository
uses: actions/[email protected]
-- invalid-version/.github/workflows/gha.sum --
version 0

actions/[email protected] GGAV+/JnlPt41B9iINyvcX5z6a4ue+NblmwiDNVORz0=
-- invalid-version/.github/workflows/workflow.yml --
name: Example workflow
on: [push]

jobs:
example:
name: example
runs-on: ubuntu-22.04
steps:
- name: Checkout repository
uses: actions/[email protected]
-- nan-version/.github/workflows/gha.sum --
version not-a-number

actions/[email protected] GGAV+/JnlPt41B9iINyvcX5z6a4ue+NblmwiDNVORz0=
-- nan-version/.github/workflows/workflow.yml --
name: Example workflow
on: [push]

jobs:
example:
name: example
runs-on: ubuntu-22.04
steps:
- name: Checkout repository
uses: actions/[email protected]
-- no-version/.github/workflows/gha.sum --
version-header-missing 1

actions/[email protected] GGAV+/JnlPt41B9iINyvcX5z6a4ue+NblmwiDNVORz0=
-- no-version/.github/workflows/workflow.yml --
name: Example workflow
on: [push]

jobs:
example:
name: example
runs-on: ubuntu-22.04
steps:
- name: Checkout repository
uses: actions/[email protected]
-- .cache/actions/checkout/v4.1.1/.keep --
This file exist to avoid fetching "actions/[email protected]" and give the Action
a unique checksum.
-- .want/gha.sum --
version 1

actions/[email protected] KsR9XQGH7ydTl01vlD8pIZrXhkzXyjcnzhmP+/KaJZI=