Add go vet and staticcheck to CI (#183)

ethan-lowman-dd · web-flow · commit 7ded50136bf9 · 2021-12-03T16:00:25.000-05:00
* Add go vet and staticcheck to CI Fix go vet bugs Make staticcheck happy Add to go.mod Revert "Add to go.mod" This reverts commit 292d57f. Specify staticcheck version Try without go.mod Attempt Attempt Attempt Fail early Fail early Attempt Attempt Attempt Attempt * Undo accidental upgrade * unused var * Use assert library * Remove special case for 1.15
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -33,3 +33,9 @@ jobs:
       run: |
         GO111MODULE=off go get github.com/mattn/goveralls
         $(go env GOPATH)/bin/goveralls -coverprofile=profile.cov -service=github
+    - name: Vet
+      run: go vet ./...
+    - name: Install staticcheck
+      run: "go install honnef.co/go/tools/cmd/staticcheck@v0.2.2"
+    - name: Run staticcheck
+      run: staticcheck ./...
diff --git a/client/client.go b/client/client.go
@@ -3,7 +3,6 @@ package client
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"io"
 	"io/ioutil"
 	"log"
@@ -560,56 +559,6 @@ func (c *Client) downloadMetaUnsafe(name string, maxMetaSize int64) ([]byte, err
 	return ioutil.ReadAll(io.LimitReader(r, maxMetaSize))
 }
 
-// getRootAndLocalVersionsUnsafe decodes the versions stored in the local
-// metadata without verifying signatures to protect against downgrade attacks
-// when the root is replaced and contains new keys. It also sets the local meta
-// cache to only contain the local root metadata.
-func (c *Client) getRootAndLocalVersionsUnsafe() error {
-	type versionData struct {
-		Signed struct {
-			Version int
-		}
-	}
-
-	meta, err := c.local.GetMeta()
-	if err != nil {
-		return err
-	}
-
-	getVersion := func(name string) (int, error) {
-		m, ok := meta[name]
-		if !ok {
-			return 0, nil
-		}
-		var data versionData
-		if err := json.Unmarshal(m, &data); err != nil {
-			return 0, err
-		}
-		return data.Signed.Version, nil
-	}
-
-	c.timestampVer, err = getVersion("timestamp.json")
-	if err != nil {
-		return err
-	}
-	c.snapshotVer, err = getVersion("snapshot.json")
-	if err != nil {
-		return err
-	}
-	c.targetsVer, err = getVersion("targets.json")
-	if err != nil {
-		return err
-	}
-
-	root, ok := meta["root.json"]
-	if !ok {
-		return errors.New("tuf: missing local root after downloading, this should not be possible")
-	}
-	c.localMeta = map[string]json.RawMessage{"root.json": root}
-
-	return nil
-}
-
 // remoteGetFunc is the type of function the download method uses to download
 // remote files
 type remoteGetFunc func(string) (io.ReadCloser, int64, error)
@@ -790,6 +739,7 @@ func (c *Client) localMetaFromSnapshot(name string, m data.SnapshotFileMeta) (js
 }
 
 // hasTargetsMeta checks whether local metadata has the given snapshot meta
+//lint:ignore U1000 unused
 func (c *Client) hasTargetsMeta(m data.SnapshotFileMeta) bool {
 	b, ok := c.localMeta["targets.json"]
 	if !ok {
@@ -804,6 +754,7 @@ func (c *Client) hasTargetsMeta(m data.SnapshotFileMeta) bool {
 }
 
 // hasSnapshotMeta checks whether local metadata has the given meta
+//lint:ignore U1000 unused
 func (c *Client) hasMetaFromTimestamp(name string, m data.TimestampFileMeta) bool {
 	b, ok := c.localMeta[name]
 	if !ok {
diff --git a/client/client_test.go b/client/client_test.go
@@ -84,7 +84,7 @@ func (f *fakeFile) Read(p []byte) (int, error) {
 }
 
 func (f *fakeFile) Close() error {
-	f.buf.Seek(0, os.SEEK_SET)
+	f.buf.Seek(0, io.SeekStart)
 	return nil
 }
 
@@ -905,14 +905,21 @@ func (s *ClientSuite) TestUpdateReplayAttack(c *C) {
 	c.Assert(s.repo.Timestamp(), IsNil)
 	s.syncRemote(c)
 	_, err := client.Update()
+	c.Assert(err, IsNil)
 	c.Assert(client.timestampVer > version, Equals, true)
 
 	// replace remote timestamp.json with the old one
 	s.remote.meta["timestamp.json"] = oldTimestamp
 
 	// check update returns ErrLowVersion
 	_, err = client.Update()
-	c.Assert(err, DeepEquals, ErrDecodeFailed{"timestamp.json", verify.ErrLowVersion{version, client.timestampVer}})
+	c.Assert(err, DeepEquals, ErrDecodeFailed{
+		File: "timestamp.json",
+		Err: verify.ErrLowVersion{
+			Actual:  version,
+			Current: client.timestampVer,
+		},
+	})
 }
 
 func (s *ClientSuite) TestUpdateTamperedTargets(c *C) {
@@ -1109,7 +1116,7 @@ func (s *ClientSuite) TestUnknownKeyIDs(c *C) {
 
 	var root struct {
 		Signed     data.Root        `json:"signed"`
-		Signatures []data.Signature `json:signatures"`
+		Signatures []data.Signature `json:"signatures"`
 	}
 	c.Assert(json.Unmarshal(rootJSON, &root), IsNil)
 
@@ -1137,6 +1144,7 @@ func (s *ClientSuite) TestUnknownKeyIDs(c *C) {
 	// the TUF-0.9 update workflow, where we decide to update the root
 	// metadata when we observe a new root through the snapshot.
 	repo, err := tuf.NewRepo(s.store)
+	c.Assert(err, IsNil)
 	c.Assert(repo.Snapshot(), IsNil)
 	c.Assert(repo.Timestamp(), IsNil)
 	c.Assert(repo.Commit(), IsNil)
diff --git a/client/delegations_test.go b/client/delegations_test.go
@@ -263,6 +263,7 @@ func initTestDelegationClient(t *testing.T, dirPrefix string) (*Client, func() e
 		TargetsPath:  "targets",
 	}
 	remote, err := HTTPRemoteStore(fmt.Sprintf("http://%s/", addr), opts, nil)
+	assert.Nil(t, err)
 
 	c := NewClient(MemoryLocalStore(), remote)
 	rawFile, err := ioutil.ReadFile(initialStateDir + "/" + "root.json")
diff --git a/client/errors.go b/client/errors.go
@@ -49,6 +49,7 @@ func (e ErrMaxDelegations) Error() string {
 	return fmt.Sprintf("tuf: max delegation of %d reached searching for %s with snapshot version %d", e.MaxDelegations, e.Target, e.SnapshotVersion)
 }
 
+//lint:ignore U1000 unused
 func isDecodeFailedWithErrRoleThreshold(err error) bool {
 	e, ok := err.(ErrDecodeFailed)
 	if !ok {
diff --git a/client/interop_test.go b/client/interop_test.go
@@ -53,6 +53,10 @@ func computeHashes(c *C, dir string) map[string]string {
 	hashes := make(map[string]string)
 
 	err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
 		if info.IsDir() {
 			return nil
 		}
diff --git a/cmd/tuf-client/get.go b/cmd/tuf-client/get.go
@@ -44,7 +44,7 @@ func cmdGet(args *docopt.Args, client *tuf.Client) error {
 		return err
 	}
 	defer tmp.Delete()
-	if _, err := tmp.Seek(0, os.SEEK_SET); err != nil {
+	if _, err := tmp.Seek(0, io.SeekStart); err != nil {
 		return err
 	}
 	_, err = io.Copy(os.Stdout, file)
diff --git a/cmd/tuf/main.go b/cmd/tuf/main.go
@@ -14,7 +14,7 @@ import (
 	docopt "github.com/flynn/go-docopt"
 	tuf "github.com/theupdateframework/go-tuf"
 	"github.com/theupdateframework/go-tuf/util"
-	"golang.org/x/crypto/ssh/terminal"
+	"golang.org/x/term"
 )
 
 func main() {
@@ -143,7 +143,7 @@ func getPassphrase(role string, confirm bool, change bool) ([]byte, error) {
 		role = fmt.Sprintf("new %s", role)
 	}
 	fmt.Printf("Enter %s keys passphrase: ", role)
-	passphrase, err := terminal.ReadPassword(int(syscall.Stdin))
+	passphrase, err := term.ReadPassword(int(syscall.Stdin))
 	fmt.Println()
 	if err != nil {
 		return nil, err
@@ -154,14 +154,14 @@ func getPassphrase(role string, confirm bool, change bool) ([]byte, error) {
 	}
 
 	fmt.Printf("Repeat %s keys passphrase: ", role)
-	confirmation, err := terminal.ReadPassword(int(syscall.Stdin))
+	confirmation, err := term.ReadPassword(int(syscall.Stdin))
 	fmt.Println()
 	if err != nil {
 		return nil, err
 	}
 
 	if !bytes.Equal(passphrase, confirmation) {
-		return nil, errors.New("The entered passphrases do not match")
+		return nil, errors.New("the entered passphrases do not match")
 	}
 	return passphrase, nil
 }
diff --git a/go.mod b/go.mod
@@ -9,5 +9,6 @@ require (
 	github.com/stretchr/testify v1.7.0
 	github.com/syndtr/goleveldb v1.0.0
 	golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871
+	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
 	gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405
 )
diff --git a/go.sum b/go.sum
@@ -35,9 +35,12 @@ golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5h
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4=
+golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4=
+golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
+golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
diff --git a/internal/signer/sort_test.go b/internal/signer/sort_test.go
@@ -16,12 +16,10 @@ type mockSigner struct {
 
 func (s *mockSigner) MarshalPrivateKey() (*data.PrivateKey, error) {
 	panic("not implemented")
-	return nil, nil
 }
 
 func (s *mockSigner) UnmarshalPrivateKey(key *data.PrivateKey) error {
 	panic("not implemented")
-	return nil
 }
 
 func (s *mockSigner) PublicData() *data.PublicKey {
@@ -34,7 +32,6 @@ func (s *mockSigner) PublicData() *data.PublicKey {
 }
 func (s *mockSigner) SignMessage(message []byte) ([]byte, error) {
 	panic("not implemented")
-	return nil, nil
 }
 
 func TestSignerSortByIDs(t *testing.T) {
diff --git a/local_store.go b/local_store.go
@@ -301,6 +301,7 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
 			return err
 		}
 		if !info.IsDir() && isTarget(rel) && needsRemoval(rel) {
+			//lint:ignore SA9003 empty branch
 			if err := os.Remove(path); err != nil {
 				// TODO: log / handle error
 			}
diff --git a/repo.go b/repo.go
@@ -409,7 +409,7 @@ func (r *Repo) AddVerificationKeyWithExpiration(keyRole string, pk *data.PublicK
 }
 
 func validExpires(expires time.Time) bool {
-	return expires.Sub(time.Now()) > 0
+	return time.Until(expires) > 0
 }
 
 func (r *Repo) RootKeys() ([]*data.PublicKey, error) {
diff --git a/repo_test.go b/repo_test.go
@@ -22,7 +22,6 @@ import (
 	"github.com/theupdateframework/go-tuf/util"
 	"github.com/theupdateframework/go-tuf/verify"
 	"golang.org/x/crypto/ed25519"
-	"gopkg.in/check.v1"
 	. "gopkg.in/check.v1"
 )
 
@@ -1314,7 +1313,7 @@ func (rs *RepoSuite) TestKeyPersistence(c *C) {
 				}
 			}
 
-			c.Assert(numMatches, Equals, expectedNumMatches, check.Commentf("actual: %+v, expected: %+v", actual, expected))
+			c.Assert(numMatches, Equals, expectedNumMatches, Commentf("actual: %+v, expected: %+v", actual, expected))
 		}
 
 		// check GetKeys is correct
@@ -1341,7 +1340,7 @@ func (rs *RepoSuite) TestKeyPersistence(c *C) {
 				}
 			}
 
-			c.Assert(numMatches, Equals, expectedNumMatches, check.Commentf("signers: %+v, expected: %+v", signers, expected))
+			c.Assert(numMatches, Equals, expectedNumMatches, Commentf("signers: %+v, expected: %+v", signers, expected))
 		}
 	}
 
@@ -1399,7 +1398,7 @@ func (rs *RepoSuite) TestKeyPersistence(c *C) {
 	// 6. Try to add a key and implicitly decrypt the keys file using the OLD passphrase - should FAIL
 	newKey, err = keys.GenerateEd25519Key()
 	c.Assert(err, IsNil)
-	newPrivateKey, err = newKey.MarshalPrivateKey()
+	_, err = newKey.MarshalPrivateKey()
 	c.Assert(err, IsNil)
 	c.Assert(store.SaveSigner("root", newKey), NotNil)
 
@@ -1554,7 +1553,7 @@ func (rs *RepoSuite) TestUnknownKeyIDs(c *C) {
 
 	var signedRoot struct {
 		Signed     data.Root        `json:"signed"`
-		Signatures []data.Signature `json:signatures"`
+		Signatures []data.Signature `json:"signatures"`
 	}
 	c.Assert(json.Unmarshal(rootJSON, &signedRoot), IsNil)
 	c.Assert(signedRoot.Signed.Version, Equals, 1)
@@ -1565,6 +1564,8 @@ func (rs *RepoSuite) TestUnknownKeyIDs(c *C) {
 
 	// a new root should preserve the unknown key id.
 	root, err = r.root()
+	c.Assert(root, NotNil)
+	c.Assert(err, IsNil)
 
 	genKey(c, r, "timestamp")
 	c.Assert(r.Snapshot(), IsNil)
diff --git a/util/util.go b/util/util.go
@@ -215,23 +215,31 @@ func GenerateSnapshotFileMeta(r io.Reader, hashAlgorithms ...string) (data.Snaps
 	if err != nil {
 		return data.SnapshotFileMeta{}, err
 	}
-	return data.SnapshotFileMeta{m, v}, nil
+	return data.SnapshotFileMeta{
+		FileMeta: m,
+		Version:  v,
+	}, nil
 }
 
 func GenerateTargetFileMeta(r io.Reader, hashAlgorithms ...string) (data.TargetFileMeta, error) {
 	m, err := GenerateFileMeta(r, hashAlgorithms...)
 	if err != nil {
 		return data.TargetFileMeta{}, err
 	}
-	return data.TargetFileMeta{m}, nil
+	return data.TargetFileMeta{
+		FileMeta: m,
+	}, nil
 }
 
 func GenerateTimestampFileMeta(r io.Reader, hashAlgorithms ...string) (data.TimestampFileMeta, error) {
 	m, v, err := generateVersionedFileMeta(r, hashAlgorithms...)
 	if err != nil {
 		return data.TimestampFileMeta{}, err
 	}
-	return data.TimestampFileMeta{m, v}, nil
+	return data.TimestampFileMeta{
+		FileMeta: m,
+		Version:  v,
+	}, nil
 }
 
 func NormalizeTarget(p string) string {
diff --git a/util/util_test.go b/util/util_test.go
@@ -115,11 +115,11 @@ func (UtilSuite) TestSnapshotFileMetaEqual(c *C) {
 
 	fileMeta := func(version int, length int64, hashes map[string]string) data.SnapshotFileMeta {
 		return data.SnapshotFileMeta{
-			data.FileMeta{
+			FileMeta: data.FileMeta{
 				Length: length,
 				Hashes: makeHashes(c, hashes),
 			},
-			version,
+			Version: version,
 		}
 	}
 
diff --git a/verify/verify.go b/verify/verify.go
@@ -28,7 +28,7 @@ func (db *DB) VerifyIgnoreExpiredCheck(s *data.Signed, role string, minVersion i
 	if isTopLevelRole(role) {
 		// Top-level roles can only sign metadata of the same type (e.g. snapshot
 		// metadata must be signed by the snapshot role).
-		if strings.ToLower(sm.Type) != strings.ToLower(role) {
+		if !strings.EqualFold(sm.Type, role) {
 			return ErrWrongMetaType
 		}
 	} else {
@@ -66,7 +66,7 @@ func (db *DB) Verify(s *data.Signed, role string, minVersion int) error {
 }
 
 var IsExpired = func(t time.Time) bool {
-	return t.Sub(time.Now()) <= 0
+	return time.Until(t) <= 0
 }
 
 func (db *DB) VerifySignatures(s *data.Signed, role string) error {

Original file line number	Diff line number	Diff line change
`@@ -263,6 +263,7 @@ func initTestDelegationClient(t testing.T, dirPrefix string) (Client, func() e`
`263`	`263`	`TargetsPath: "targets",`
`264`	`264`	`}`
`265`	`265`	`remote, err := HTTPRemoteStore(fmt.Sprintf("http://%s/", addr), opts, nil)`
	`266`	`+ assert.Nil(t, err)`
`266`	`267`
`267`	`268`	`c := NewClient(MemoryLocalStore(), remote)`
`268`	`269`	`rawFile, err := ioutil.ReadFile(initialStateDir + "/" + "root.json")`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ func (e ErrMaxDelegations) Error() string {`
`49`	`49`	`return fmt.Sprintf("tuf: max delegation of %d reached searching for %s with snapshot version %d", e.MaxDelegations, e.Target, e.SnapshotVersion)`
`50`	`50`	`}`
`51`	`51`
	`52`	`+//lint:ignore U1000 unused`
`52`	`53`	`func isDecodeFailedWithErrRoleThreshold(err error) bool {`
`53`	`54`	`e, ok := err.(ErrDecodeFailed)`
`54`	`55`	`if !ok {`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ func cmdGet(args docopt.Args, client tuf.Client) error {`
`44`	`44`	`return err`
`45`	`45`	`}`
`46`	`46`	`defer tmp.Delete()`
`47`		`- if _, err := tmp.Seek(0, os.SEEK_SET); err != nil {`
	`47`	`+ if _, err := tmp.Seek(0, io.SeekStart); err != nil {`
`48`	`48`	`return err`
`49`	`49`	`}`
`50`	`50`	`_, err = io.Copy(os.Stdout, file)`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ import (`
`14`	`14`	`docopt "github.com/flynn/go-docopt"`
`15`	`15`	`tuf "github.com/theupdateframework/go-tuf"`
`16`	`16`	`"github.com/theupdateframework/go-tuf/util"`
`17`		`- "golang.org/x/crypto/ssh/terminal"`
	`17`	`+ "golang.org/x/term"`
`18`	`18`	`)`
`19`	`19`
`20`	`20`	`func main() {`
`@@ -143,7 +143,7 @@ func getPassphrase(role string, confirm bool, change bool) ([]byte, error) {`
`143`	`143`	`role = fmt.Sprintf("new %s", role)`
`144`	`144`	`}`
`145`	`145`	`fmt.Printf("Enter %s keys passphrase: ", role)`
`146`		`- passphrase, err := terminal.ReadPassword(int(syscall.Stdin))`
	`146`	`+ passphrase, err := term.ReadPassword(int(syscall.Stdin))`
`147`	`147`	`fmt.Println()`
`148`	`148`	`if err != nil {`
`149`	`149`	`return nil, err`
`@@ -154,14 +154,14 @@ func getPassphrase(role string, confirm bool, change bool) ([]byte, error) {`
`154`	`154`	`}`
`155`	`155`
`156`	`156`	`fmt.Printf("Repeat %s keys passphrase: ", role)`
`157`		`- confirmation, err := terminal.ReadPassword(int(syscall.Stdin))`
	`157`	`+ confirmation, err := term.ReadPassword(int(syscall.Stdin))`
`158`	`158`	`fmt.Println()`
`159`	`159`	`if err != nil {`
`160`	`160`	`return nil, err`
`161`	`161`	`}`
`162`	`162`
`163`	`163`	`if !bytes.Equal(passphrase, confirmation) {`
`164`		`- return nil, errors.New("The entered passphrases do not match")`
	`164`	`+ return nil, errors.New("the entered passphrases do not match")`
`165`	`165`	`}`
`166`	`166`	`return passphrase, nil`
`167`	`167`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,5 +9,6 @@ require (`
`9`	`9`	`github.com/stretchr/testify v1.7.0`
`10`	`10`	`github.com/syndtr/goleveldb v1.0.0`
`11`	`11`	`golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871`
	`12`	`+ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211`
`12`	`13`	`gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405`
`13`	`14`	`)`