fix: a lot of lint issues (#124)

* fix: a lot of lint issues fixes a couple of missing error handlings, security issues, and ignores a bunch of client closing error handling warnings Signed-off-by: Carlos A Becker <[email protected]> * fix: more linting fixes Signed-off-by: Carlos A Becker <[email protected]> Co-authored-by: Ayman Bagabas <[email protected]>
charmbracelet · Apr 22, 2022 · d060ca1 · d060ca1
1 parent c8b679f
commit d060ca1
Show file tree

Hide file tree

Showing 33 changed files with 226 additions and 184 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -32,3 +32,8 @@ linters:
     - unconvert
     - unparam
     - whitespace
+
+rules:
+  - linters:
+    - godox
+    severity: warn
diff --git a/client/auth.go b/client/auth.go
@@ -19,7 +19,7 @@ func (cc *Client) Auth() (*charm.Auth, error) {
 		if err != nil {
 			return nil, charm.ErrAuthFailed{Err: err}
 		}
-		defer s.Close()
+		defer s.Close() // nolint:errcheck
 
 		b, err := s.Output("api-auth")
 		if err != nil {

diff --git a/client/client.go b/client/client.go
@@ -109,6 +109,7 @@ func NewClient(cfg *Config) (*Client, error) {
 	return cc, nil
 }
 
+// NewClientWithDefaults creates a new Charm client with default values.
 func NewClientWithDefaults() (*Client, error) {
 	cfg, err := ConfigFromEnv()
 	if err != nil {
@@ -127,7 +128,7 @@ func (cc *Client) JWT(aud ...string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	jwt, err := s.Output(strings.Join(append([]string{"jwt"}, aud...), " "))
 	if err != nil {
 		return "", err
@@ -141,7 +142,7 @@ func (cc *Client) ID() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	id, err := s.Output("id")
 	if err != nil {
 		return "", err
@@ -155,7 +156,7 @@ func (cc *Client) AuthorizedKeys() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	keys, err := s.Output("keys")
 	if err != nil {
 		return "", err
@@ -169,7 +170,7 @@ func (cc *Client) AuthorizedKeysWithMetadata() (*charm.Keys, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 
 	b, err := s.Output("api-keys")
 	if err != nil {
@@ -192,7 +193,7 @@ func (cc *Client) UnlinkAuthorizedKey(key string) error {
 	if err != nil {
 		return err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	k := charm.PublicKey{Key: key}
 	in, err := s.StdinPipe()
 	if err != nil {
@@ -215,6 +216,7 @@ func (cc *Client) UnlinkAuthorizedKey(key string) error {
 	return nil
 }
 
+// KeygenType returns the keygen key type.
 func (cfg *Config) KeygenType() keygen.KeyType {
 	switch cfg.KeyType {
 	case "Ed25519", "ed25519":
@@ -280,14 +282,14 @@ func (cc *Client) sshSession() (*ssh.Session, error) {
 func (cc *Client) DataPath() (string, error) {
 	if cc.Config.DataDir != "" {
 		return filepath.Join(cc.Config.DataDir, cc.Config.Host), nil
-	} else {
-		scope := gap.NewScope(gap.User, filepath.Join("charm", cc.Config.Host))
-		dataPath, err := scope.DataPath("")
-		if err != nil {
-			return "", err
-		}
-		return dataPath, nil
 	}
+
+	scope := gap.NewScope(gap.User, filepath.Join("charm", cc.Config.Host))
+	dataPath, err := scope.DataPath("")
+	if err != nil {
+		return "", err
+	}
+	return dataPath, nil
 }
 
 // FindAuthKeys looks in a user's XDG charm-dir for possible auth keys.

diff --git a/client/crypt.go b/client/crypt.go
@@ -58,8 +58,7 @@ func (cc *Client) findIdentities() ([]sasquatch.Identity, error) {
 
 // EncryptKeys returns all of the symmetric encrypt keys for the authed user.
 func (cc *Client) EncryptKeys() ([]*charm.EncryptKey, error) {
-	err := cc.cryptCheck()
-	if err != nil {
+	if err := cc.cryptCheck(); err != nil {
 		return nil, err
 	}
 	return cc.plainTextEncryptKeys, nil
@@ -75,8 +74,12 @@ func (cc *Client) addEncryptKey(pk string, gid string, key string, createdAt *ti
 	if err != nil {
 		return err
 	}
-	w.Write([]byte(key))
-	w.Close()
+	if _, err := w.Write([]byte(key)); err != nil {
+		return err
+	}
+	if err := w.Close(); err != nil {
+		return err
+	}
 
 	encKey := base64.StdEncoding.EncodeToString(buf.Bytes())
 	ek := charm.EncryptKey{}

diff --git a/client/http.go b/client/http.go
@@ -37,7 +37,7 @@ func (cc *Client) AuthedJSONRequest(method string, path string, reqBody interfac
 		return err
 	}
 	if respBody != nil {
-		defer resp.Body.Close()
+		defer resp.Body.Close() // nolint:errcheck
 		dec := json.NewDecoder(resp.Body)
 		return dec.Decode(respBody)
 	}

diff --git a/client/link.go b/client/link.go
@@ -14,7 +14,7 @@ func (cc *Client) LinkGen(lh charm.LinkHandler) error {
 	if err != nil {
 		return err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	out, err := s.StdoutPipe()
 	if err != nil {
 		return err
@@ -83,7 +83,7 @@ func (cc *Client) Link(lh charm.LinkHandler, code string) error {
 	if err != nil {
 		return err
 	}
-	defer s.Close()
+	defer s.Close() // nolint:errcheck
 	out, err := s.StdoutPipe()
 	if err != nil {
 		return err

diff --git a/client/news.go b/client/news.go
@@ -8,6 +8,7 @@ import (
 	charm "github.com/charmbracelet/charm/proto"
 )
 
+// NewsList lists the server news.
 func (cc *Client) NewsList(tags []string, page int) ([]*charm.News, error) {
 	var nl []*charm.News
 
@@ -22,6 +23,7 @@ func (cc *Client) NewsList(tags []string, page int) ([]*charm.News, error) {
 	return nl, nil
 }
 
+// News shows a given news.
 func (cc *Client) News(id string) (*charm.News, error) {
 	var n *charm.News
 	err := cc.AuthedJSONRequest("GET", fmt.Sprintf("/v1/news/%s", url.QueryEscape(id)), nil, &n)

diff --git a/cmd/backup_keys.go b/cmd/backup_keys.go
@@ -99,7 +99,6 @@ func validateDirectory(path string) error {
 
 		// Everything looks OK!
 		return nil
-
 	} else if os.IsNotExist(err) {
 		return fmt.Errorf("'%v' does not exist", path)
 	} else {
@@ -112,10 +111,10 @@ func createTar(source string, target string) error {
 	if err != nil {
 		return err
 	}
-	defer tarfile.Close()
+	defer tarfile.Close() // nolint:errcheck
 
 	tarball := tar.NewWriter(tarfile)
-	defer tarball.Close()
+	defer tarball.Close() // nolint:errcheck
 
 	info, err := os.Stat(source)
 	if err != nil {
@@ -158,7 +157,7 @@ func createTar(source string, target string) error {
 			if err != nil {
 				return err
 			}
-			defer file.Close()
+			defer file.Close() // nolint:errcheck
 			_, err = io.Copy(tarball, file)
 			return err
 		})

diff --git a/cmd/crypt.go b/cmd/crypt.go
@@ -76,7 +76,7 @@ func cryptEncrypt(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	eb.Close()
+	eb.Close() // nolint:errcheck
 	cf := cryptFile{
 		Data: base64.StdEncoding.EncodeToString(buf.Bytes()),
 	}
@@ -99,7 +99,7 @@ func cryptDecrypt(cmd *cobra.Command, args []string) error {
 		r = os.Stdin
 	default:
 		f, err := os.Open(args[0])
-		defer f.Close()
+		defer f.Close() // nolint:errcheck
 		r = f
 		if err != nil {
 			return err

diff --git a/cmd/fs.go b/cmd/fs.go
@@ -39,7 +39,7 @@ var (
 		Use:    "fs",
 		Hidden: false,
 		Short:  "Use the Charm file system.",
-		Long:   paragraph(fmt.Sprintf("Commands to set, get and delete data from your Charm Cloud backed file system.")),
+		Long:   paragraph("Commands to set, get and delete data from your Charm Cloud backed file system."),
 		Args:   cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return nil
@@ -120,12 +120,7 @@ func newLocalRemotePath(rawPath string) localRemotePath {
 }
 
 func (lrp *localRemotePath) separator() string {
-	switch lrp.pathType {
-	case localPath:
-		return string(os.PathSeparator)
-	default:
-		return "/"
-	}
+	return "/"
 }
 
 func (lrfs *localRemoteFS) Open(name string) (fs.File, error) {
@@ -173,7 +168,7 @@ func (lrfs *localRemoteFS) write(name string, src fs.File) error {
 			if err != nil {
 				return err
 			}
-			defer f.Close()
+			defer f.Close() // nolint:errcheck
 			_, err = io.Copy(f, src)
 			if err != nil {
 				return err
@@ -194,7 +189,7 @@ func (lrfs *localRemoteFS) copy(srcName string, dstName string, recursive bool)
 	if err != nil {
 		return err
 	}
-	defer src.Close()
+	defer src.Close() // nolint:errcheck
 	stat, err := src.Stat()
 	if err != nil {
 		return err
@@ -216,7 +211,7 @@ func (lrfs *localRemoteFS) copy(srcName string, dstName string, recursive bool)
 			if err != nil {
 				return err
 			}
-			defer wsrc.Close()
+			defer wsrc.Close() // nolint:errcheck
 			wp := newLocalRemotePath(wps)
 			wpp := strings.Split(filepath.Clean(wp.path), wp.separator())
 			rp := path.Join(wpp[parents:]...)
@@ -235,17 +230,20 @@ func fsCat(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer f.Close() // nolint:errcheck
+
 	fi, err := f.Stat()
 	if err != nil {
 		return err
 	}
+
 	if fi.IsDir() {
 		fmt.Printf("cat: %s: Is a directory\n", args[0])
-	} else {
-		io.Copy(os.Stdout, f)
+		return nil
 	}
-	return nil
+
+	_, err = io.Copy(os.Stdout, f)
+	return err
 }
 
 func fsMove(cmd *cobra.Command, args []string) error {
@@ -297,7 +295,7 @@ func fsList(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer f.Close() // nolint:errcheck
 	fi, err := f.Stat()
 	if err != nil {
 		return err
@@ -350,8 +348,7 @@ func printDir(f fs.ReadDirFile) error {
 		}
 		fprintFileInfo(w, dfi)
 	}
-	w.Flush()
-	return nil
+	return w.Flush()
 }
 
 func init() {

diff --git a/cmd/import_keys.go b/cmd/import_keys.go
@@ -52,7 +52,7 @@ var (
 				return err
 			}
 
-			if err := os.MkdirAll(dd, 0700); err != nil {
+			if err := os.MkdirAll(dd, 0o700); err != nil {
 				return err
 			}
 
@@ -160,7 +160,7 @@ func isEmpty(name string) (bool, error) {
 	if err != nil {
 		return false, err
 	}
-	defer f.Close()
+	defer f.Close() // nolint:errcheck
 
 	_, err = f.Readdirnames(1)
 	if err == io.EOF {
@@ -174,7 +174,7 @@ func untar(tarball, targetDir string) error {
 	if err != nil {
 		return err
 	}
-	defer reader.Close()
+	defer reader.Close() // nolint:errcheck
 	tarReader := tar.NewReader(reader)
 
 	for {
@@ -208,10 +208,16 @@ func untar(tarball, targetDir string) error {
 		if err != nil {
 			return err
 		}
-		defer file.Close()
-		_, err = io.Copy(file, tarReader)
-		if err != nil {
-			return err
+		defer file.Close() // nolint:errcheck
+
+		for {
+			_, err := io.CopyN(file, tarReader, 1024)
+			if err != nil {
+				if err == io.EOF {
+					break
+				}
+				return err
+			}
 		}
 	}
 	return nil

diff --git a/cmd/keys.go b/cmd/keys.go
@@ -10,8 +10,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var randomart bool
-var simpleOutput bool
+var (
+	randomart    bool
+	simpleOutput bool
+)
 
 // KeysCmd is the cobra.Command for a user to browser and print their linked
 // SSH keys.
@@ -29,7 +31,7 @@ var KeysCmd = &cobra.Command{
 				if err != nil {
 					return err
 				}
-				defer f.Close()
+				defer f.Close() // nolint:errcheck
 			}
 			return keys.NewProgram(cfg).Start()
 		}