chore: address fix linter findings (#607)

Issue #, if available: *Description of changes:* Address all linter findings, including ignoring gosec warnings about auditing the use of unsafe pointers in pkg/winutil/run_windows.go https://securego.io/docs/rules/g103.html *Testing done:* `make lint` - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Gavin Inglis <[email protected]>
runfinch · Oct 6, 2023 · fedffba · fedffba
1 parent 2657d93
commit fedffba
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 71 deletions.
diff --git a/cmd/finch/nerdctl_windows.go b/cmd/finch/nerdctl_windows.go
@@ -267,20 +267,19 @@ func cpHandler(systemDeps NerdctlCommandSystemDeps, nerdctlCmdArgs []string) err
 		// -L and --follow-symlink don't have to be processed
 		if strings.HasPrefix(arg, "-") || arg == "cp" {
 			continue
-		} else {
-			// If argument contains container path, then continue
-			colon := strings.Index(arg, ":")
+		}
+		// If argument contains container path, then continue
+		colon := strings.Index(arg, ":")
 
-			// this is a container path
-			if colon > 1 {
-				continue
-			}
-			wslPath, err := convertToWSLPath(systemDeps, arg)
-			if err != nil {
-				return err
-			}
-			nerdctlCmdArgs[i] = wslPath
+		// this is a container path
+		if colon > 1 {
+			continue
+		}
+		wslPath, err := convertToWSLPath(systemDeps, arg)
+		if err != nil {
+			return err
 		}
+		nerdctlCmdArgs[i] = wslPath
 	}
 	return nil
 }

diff --git a/cmd/finch/nerdctl_windows_test.go b/cmd/finch/nerdctl_windows_test.go
@@ -120,7 +120,8 @@ func TestNerdctlCommand_run(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, build => image build
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "image", "build", "-t", "demo", "/mnt/c/Users").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "image", "build", "-t", "demo", "/mnt/c/Users").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -255,9 +256,8 @@ func TestNerdctlCommand_run(t *testing.T) {
 				ncsd.EXPECT().FilePathAbs("C:\\workdir").Return("C:\\workdir", nil)
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
 				ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
-				lcc.EXPECT().
-					Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env").
-					Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG1=val1", "--rm", "alpine:latest", "env").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -292,9 +292,8 @@ func TestNerdctlCommand_run(t *testing.T) {
 				ncsd.EXPECT().FilePathAbs("C:\\workdir").Return("C:\\workdir", nil)
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
 				ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
-				lcc.EXPECT().
-					Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env").
-					Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "-e", "ARG2=val2", "--rm", "alpine:latest", "env").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -536,7 +535,8 @@ func TestNerdctlCommand_run(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir").Return(augmentedPath)
 				ncsd.EXPECT().FilePathToSlash(augmentedPath).Return(wslPath)
 				lcc.EXPECT().RunWithReplacingStdout(
-					testStdoutRs, "shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil)
+					testStdoutRs, "shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil)
 			},
 		},
 		{
@@ -737,7 +737,9 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, run => container run
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", ContainsStr("src=/mnt/c/workdir"), "alpine:latest").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "--mount",
+					ContainsStr("src=/mnt/c/workdir"), "alpine:latest").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -769,7 +771,9 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, run => container run
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", ContainsStr("source=/mnt/c/workdir"), "alpine:latest").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "--mount",
+					ContainsStr("source=/mnt/c/workdir"), "alpine:latest").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -801,7 +805,9 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, run => container run
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", ContainsStr("source=/mnt/c/workdir"), "alpine:latest").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run",
+					ContainsStr("source=/mnt/c/workdir"), "alpine:latest").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -833,7 +839,9 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, run => container run
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", ContainsStr("source=something"), "alpine:latest").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "--mount",
+					ContainsStr("source=something"), "alpine:latest").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -865,7 +873,9 @@ func TestNerdctlCommand_Run_withBindMounts(t *testing.T) {
 
 				c := mocks.NewCommand(ctrl)
 				// alias substitution, run => container run
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", "--mount", "type=notbind,source=C:/workdir,target=/app", "alpine:latest").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "run", "--mount",
+					"type=notbind,source=C:/workdir,target=/app", "alpine:latest").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -920,7 +930,8 @@ func TestNerdctlCommand_run_CpCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\test").Return(hostcopyPath)
 				ncsd.EXPECT().FilePathToSlash(hostcopyPath).Return(wslcopyPath)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "cp", wslcopyPath, "somecontainer:/tmp").Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "cp", wslcopyPath, "somecontainer:/tmp").Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -946,7 +957,8 @@ func TestNerdctlCommand_run_CpCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\test").Return(hostcopyPath)
 				ncsd.EXPECT().FilePathToSlash(hostcopyPath).Return(wslcopyPath)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "cp", "somecontainer:/tmp/test", wslcopyPath).Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "cp", "somecontainer:/tmp/test", wslcopyPath).Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -972,7 +984,8 @@ func TestNerdctlCommand_run_CpCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\test").Return(hostcopyPath)
 				ncsd.EXPECT().FilePathToSlash(hostcopyPath).Return(wslcopyPath)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "cp", "-L", "somecontainer:/tmp/test", wslcopyPath).Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "container", "cp", "-L", "somecontainer:/tmp/test", wslcopyPath).Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -1031,7 +1044,8 @@ func TestNerdctlCommand_run_BuildCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\buildcontext").Return(buildContext)
 				ncsd.EXPECT().FilePathToSlash(buildContext).Return(wslBuildContextPath)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "image", "build", wslBuildContextPath).Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "image", "build", wslBuildContextPath).Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -1057,7 +1071,8 @@ func TestNerdctlCommand_run_BuildCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\buildcontext").Return(buildContext).Times(2)
 				ncsd.EXPECT().FilePathToSlash(buildContext).Return(wslBuildContextPath).Times(2)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "image", "build", "-f", wslBuildContextPath, wslBuildContextPath).Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "image", "build", "-f", wslBuildContextPath, wslBuildContextPath).Return(c)
 				c.EXPECT().Run()
 			},
 		},
@@ -1087,7 +1102,9 @@ func TestNerdctlCommand_run_BuildCommand(t *testing.T) {
 				ncsd.EXPECT().FilePathJoin(string(filepath.Separator), "mnt", "c", "workdir\\secret").Return(secretPath)
 				ncsd.EXPECT().FilePathToSlash(secretPath).Return(wslSecretPath)
 				c := mocks.NewCommand(ctrl)
-				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName, "sudo", "-E", nerdctlCmdName, "image", "build", "--secret", fmt.Sprintf("src=%s", wslSecretPath), wslBuildContextPath).Return(c)
+				lcc.EXPECT().Create("shell", "--workdir", wslPath, limaInstanceName,
+					"sudo", "-E", nerdctlCmdName, "image", "build", "--secret",
+					fmt.Sprintf("src=%s", wslSecretPath), wslBuildContextPath).Return(c)
 				c.EXPECT().Run()
 			},
 		},

diff --git a/e2e/container/container_test.go b/e2e/container/container_test.go
@@ -43,7 +43,7 @@ func TestContainer(t *testing.T) {
 		if runtime.GOOS == "windows" {
 			// wsl2 cgroup v2 is mounted at /sys/fs/cgroup/unified,
 			// containerd expects it at /sys/fs/cgroup based on
-			//https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36
+			// https://github.com/containerd/cgroups/blob/cc78c6c1e32dc5bde018d92999910fdace3cfa27/utils.go#L36
 			tests.Run(&tests.RunOption{BaseOpt: o, CGMode: tests.Hybrid, DefaultHostGatewayIP: "192.168.5.2"})
 		} else {
 			tests.Run(&tests.RunOption{BaseOpt: o, CGMode: tests.Unified, DefaultHostGatewayIP: "192.168.5.2"})

diff --git a/pkg/config/defaults_windows.go b/pkg/config/defaults_windows.go
@@ -12,7 +12,7 @@ import (
 )
 
 // Does not matter if Rosetta is set, no-op.
-func rosettaDefault(cfg *Finch) {
+func rosettaDefault(_ *Finch) {
 }
 
 func vmDefault(cfg *Finch) {
@@ -22,9 +22,9 @@ func vmDefault(cfg *Finch) {
 }
 
 // no-op , not configurable in wsl.
-func memoryDefault(cfg *Finch, mem fmemory.Memory) {
+func memoryDefault(_ *Finch, _ fmemory.Memory) {
 }
 
 // no-op , not configurable in wsl.
-func cpuDefault(cfg *Finch, deps LoadSystemDeps) {
+func cpuDefault(_ *Finch, _ LoadSystemDeps) {
 }
diff --git a/pkg/config/validate_windows.go b/pkg/config/validate_windows.go
@@ -10,6 +10,6 @@ import (
 	"github.com/runfinch/finch/pkg/fmemory"
 )
 
-func validate(cfg *Finch, log flog.Logger, systemDeps LoadSystemDeps, mem fmemory.Memory) error {
+func validate(_ *Finch, _ flog.Logger, _ LoadSystemDeps, _ fmemory.Memory) error {
 	return nil
 }
diff --git a/pkg/disk/disk.go b/pkg/disk/disk.go
@@ -16,7 +16,7 @@ import (
 	fpath "github.com/runfinch/finch/pkg/path"
 )
 
-const DISK_SIZE = "50GB"
+const diskSizeStr = "50GB"
 
 // UserDataDiskManager is used to check the user data disk configuration and create it if needed.
 type UserDataDiskManager interface {
@@ -63,7 +63,7 @@ func NewUserDataDiskManager(
 }
 
 func diskSize() (int64, error) {
-	size, err := units.RAMInBytes(DISK_SIZE)
+	size, err := units.RAMInBytes(diskSizeStr)
 	if err != nil {
 		return 0, err
 	}

diff --git a/pkg/disk/disk_windows.go b/pkg/disk/disk_windows.go
@@ -66,18 +66,6 @@ func (m *userDataDiskManager) DetachUserDataDisk() error {
 	return nil
 }
 
-func (m *userDataDiskManager) diskExists(diskPath string) (bool, error) {
-	disksDir := filepath.Dir(diskPath)
-	_, err := m.fs.Stat(filepath.Dir(diskPath))
-	if errors.Is(err, fs.ErrNotExist) {
-		if err := m.fs.MkdirAll(disksDir, 0o755); err != nil {
-			return false, fmt.Errorf("could not create persistent disk directory: %w", err)
-		}
-	}
-
-	return true, nil
-}
-
 func (m *userDataDiskManager) createDisk(diskPath string) error {
 	size, err := sizeInMB()
 	if err != nil {

diff --git a/pkg/disk/dpgo/disk_windows.go b/pkg/disk/dpgo/disk_windows.go
@@ -46,8 +46,8 @@ func newCreateDiskCommand(ecc *command.ExecCmdCreator, logger flog.Logger, stdOu
 	createDiskCommand.Flags().StringP("path", "p", "", "the full path to the disk")
 	createDiskCommand.Flags().Int64P("size", "s", 0, "the size of the disk")
 
-	createDiskCommand.MarkFlagRequired("path")
-	createDiskCommand.MarkFlagRequired("size")
+	_ = createDiskCommand.MarkFlagRequired("path")
+	_ = createDiskCommand.MarkFlagRequired("size")
 
 	return createDiskCommand
 }
@@ -75,10 +75,8 @@ func (cd *createDiskAction) run() error {
 	if err != nil {
 		return err
 	}
-	if err := cd.createDisk(path, size); err != nil {
-		return err
-	}
-	return nil
+
+	return cd.createDisk(path, size)
 }
 
 //go:embed createDisk.TEMPLATE.txt
@@ -117,7 +115,9 @@ func (cd *createDiskAction) createDisk(path string, size int64) error {
 	go func() {
 		cd.logger.Debugf("writing to diskpart stdin: %s", tmpl.String())
 		fmt.Fprintf(dpStdin, "%s\r\n", tmpl.String())
-		dpStdin.Close()
+		if err := dpStdin.Close(); err != nil {
+			cd.logger.Debugf("failed to close diskpart stdin: %w", err)
+		}
 	}()
 
 	cd.logger.Debugln("waiting for diskpart to exit")

diff --git a/pkg/disk/dpgo/diskpart_windows.go b/pkg/disk/dpgo/diskpart_windows.go
@@ -4,6 +4,9 @@
 //go:build windows
 // +build windows
 
+// Package main is the entrypoint of the diskpart utility executable that is
+// run as a separate process from Finch such that it can be given elevated
+// Administrator privileges
 package main
 
 import (
@@ -12,7 +15,6 @@ import (
 	"io"
 	"os"
 
-	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 
 	"github.com/runfinch/finch/pkg/command"
@@ -24,14 +26,13 @@ const diskpartRootCmd = "dpgo"
 
 func main() {
 	logger := flog.NewLogrus()
-	fs := afero.NewOsFs()
 	stdOut := os.Stdout
-	if err := xmain(logger, fs, stdOut); err != nil {
+	if err := xmain(logger, stdOut); err != nil {
 		logger.Fatal(err)
 	}
 }
 
-func xmain(logger flog.Logger, fs afero.Fs, stdOut io.Writer) error {
+func xmain(logger flog.Logger, stdOut io.Writer) error {
 	rootCmd := &cobra.Command{
 		Use:           fmt.Sprintf("%v <command>", diskpartRootCmd),
 		Short:         "dpgo: utility to interact with diskpart",

diff --git a/pkg/winutil/io.go b/pkg/winutil/io.go
@@ -1,6 +1,8 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
+// Package winutil provides provides utility functions to run commands with
+// elevated Administrator privileges.
 package winutil
 
 import (

diff --git a/pkg/winutil/io_test.go b/pkg/winutil/io_test.go
@@ -14,6 +14,8 @@ import (
 )
 
 func TestFromUTF16le(t *testing.T) {
+	t.Parallel()
+
 	testCases := []struct {
 		name         string
 		r            io.Reader
@@ -49,6 +51,7 @@ func TestFromUTF16le(t *testing.T) {
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
 			tc.postRunCheck(t, FromUTF16le(tc.r))
 		})
 	}
@@ -88,9 +91,9 @@ func TestFromUTF16leToString(t *testing.T) {
 		},
 		{
 			name:         "error reading buffer",
-			r:            NewErrorReader("read error!"),
+			r:            newErrorReader("read error"),
 			postRunCheck: func(t *testing.T, str string) {},
-			wantErr:      errors.New("read error!"),
+			wantErr:      errors.New("read error"),
 		},
 	}
 
@@ -110,10 +113,10 @@ type errReader struct {
 	errMsg string
 }
 
-func (er errReader) Read(p []byte) (n int, err error) {
+func (er errReader) Read(_ []byte) (n int, err error) {
 	return 0, errors.New(er.errMsg)
 }
 
-func NewErrorReader(errMsg string) errReader {
+func newErrorReader(errMsg string) errReader {
 	return errReader{errMsg: errMsg}
 }