diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6b529f..cb1d757 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.52 + version: v1.53 args: >- --verbose --timeout=5m @@ -73,6 +73,7 @@ jobs: - go-generate runs-on: ${{ matrix.os }} strategy: + fail-fast: false matrix: os: [windows-2019, windows-2022, ubuntu-latest] steps: @@ -88,7 +89,15 @@ jobs: run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }} - name: Test repo - run: gotestsum --format standard-verbose --debug -- -gcflags=all=-d=checkptr -v ./... + run: gotestsum --format standard-verbose --debug -- -gcflags=all=-d=checkptr -race -v ./... + + # Fuzzing was added in go1.18, so all stable/supported versions of go should support it. + # hvsock fuzzing fails on windows 2019, even though tests pass. + # + # If fuzzing tests are added to different packages, add them here. + - name: Fuzz repo + if: ${{ matrix.os == 'windows-2022' }} + run: gotestsum --format standard-verbose --debug -- -run "^#" -fuzztime 500x -fuzz "FuzzHvSock" build: name: Build Repo diff --git a/.golangci.yml b/.golangci.yml index 7b503d2..483caa3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,6 +20,7 @@ linters: - gofmt # files are gofmt'ed - gosec # security - nilerr # returns nil even with non-nil error + - thelper # test helpers without t.Helper() - unparam # unused function params issues: diff --git a/backuptar/tar_test.go b/backuptar/tar_test.go index 8984c59..d6687ca 100644 --- a/backuptar/tar_test.go +++ b/backuptar/tar_test.go @@ -17,6 +17,8 @@ import ( ) func ensurePresent(t *testing.T, m map[string]string, keys ...string) { + t.Helper() + for _, k := range keys { if _, ok := m[k]; !ok { t.Error(k, "not present in tar header") @@ -25,6 +27,8 @@ func ensurePresent(t *testing.T, m map[string]string, keys ...string) { } func setSparse(t *testing.T, f *os.File) { + t.Helper() + if err := windows.DeviceIoControl(windows.Handle(f.Fd()), windows.FSCTL_SET_SPARSE, nil, 0, nil, 0, nil, nil); err != nil { t.Fatal(err) } @@ -32,6 +36,8 @@ func setSparse(t *testing.T, f *os.File) { // compareReaders validates that two readers contain the exact same data. func compareReaders(t *testing.T, rActual io.Reader, rExpected io.Reader) { + t.Helper() + const size = 8 * 1024 var bufExpected, bufActual [size]byte var readCount int64 @@ -71,6 +77,8 @@ func TestRoundTrip(t *testing.T) { //nolint:gosec // G306: Expect WriteFile permissions to be 0600 or less for name, setup := range map[string]func(*testing.T) string{ "normalFile": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") if err := os.WriteFile(path, []byte("testing 1 2 3\n"), 0644); err != nil { t.Fatal(err) @@ -78,6 +86,8 @@ func TestRoundTrip(t *testing.T) { return path }, "normalFileEmpty": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { @@ -87,6 +97,8 @@ func TestRoundTrip(t *testing.T) { return path }, "sparseFileEmpty": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { @@ -97,6 +109,8 @@ func TestRoundTrip(t *testing.T) { return path }, "sparseFileWithNoAllocatedRanges": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { @@ -112,6 +126,8 @@ func TestRoundTrip(t *testing.T) { return path }, "sparseFileWithOneAllocatedRange": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { @@ -125,6 +141,8 @@ func TestRoundTrip(t *testing.T) { return path }, "sparseFileWithMultipleAllocatedRanges": func(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "foo.txt") f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, 0644) if err != nil { diff --git a/file.go b/file.go index 175a99d..95e97da 100644 --- a/file.go +++ b/file.go @@ -21,6 +21,8 @@ import ( //sys setFileCompletionNotificationModes(h syscall.Handle, flags uint8) (err error) = SetFileCompletionNotificationModes //sys wsaGetOverlappedResult(h syscall.Handle, o *syscall.Overlapped, bytes *uint32, wait bool, flags *uint32) (err error) = ws2_32.WSAGetOverlappedResult +//todo (go1.19): switch to [atomic.Bool] + type atomicBool int32 func (b *atomicBool) isSet() bool { return atomic.LoadInt32((*int32)(b)) != 0 } diff --git a/fileinfo_test.go b/fileinfo_test.go index bdb87ed..2e2e81b 100644 --- a/fileinfo_test.go +++ b/fileinfo_test.go @@ -14,6 +14,8 @@ import ( // so we check that the current.AllocationSize is >= expected.AllocationSize. // https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/5afa7f66-619c-48f3-955f-68c4ece704ae func checkFileStandardInfo(t *testing.T, current, expected *FileStandardInfo) { + t.Helper() + if current.AllocationSize < expected.AllocationSize { t.Fatalf("FileStandardInfo unexpectedly had AllocationSize %d, expecting >=%d", current.AllocationSize, expected.AllocationSize) } diff --git a/hvsock_test.go b/hvsock_test.go index 96c4ca5..a87ff27 100644 --- a/hvsock_test.go +++ b/hvsock_test.go @@ -37,6 +37,7 @@ func serverListen(u testUtil) (l *HvsockListener, a *HvsockAddr) { u.T.Logf("address collision %v", a) continue } + u.T.Logf("listening on %v", a) break } u.Must(err, "could not listen") @@ -579,9 +580,11 @@ type testUtil struct { T testing.TB } -func newUtil(t testing.TB) testUtil { +func newUtil(tb testing.TB) testUtil { + tb.Helper() + return testUtil{ - T: t, + T: tb, } } diff --git a/pipe_test.go b/pipe_test.go index c07c06b..cb6632a 100644 --- a/pipe_test.go +++ b/pipe_test.go @@ -223,6 +223,8 @@ func TestCloseAbortsListen(t *testing.T) { } func ensureEOFOnClose(t *testing.T, r io.Reader, w io.Closer) { + t.Helper() + b := make([]byte, 10) w.Close() n, err := r.Read(b) diff --git a/pkg/bindfilter/bind_filter_test.go b/pkg/bindfilter/bind_filter_test.go index d4450e5..533df23 100644 --- a/pkg/bindfilter/bind_filter_test.go +++ b/pkg/bindfilter/bind_filter_test.go @@ -8,14 +8,15 @@ import ( "fmt" "os" "path/filepath" - "strconv" "strings" "testing" - "golang.org/x/sys/windows/registry" + "golang.org/x/sys/windows" ) func TestApplyFileBinding(t *testing.T) { + requireElevated(t) + source := t.TempDir() destination := t.TempDir() fileName := "testFile.txt" @@ -55,12 +56,15 @@ func TestApplyFileBinding(t *testing.T) { } func removeFileBinding(t *testing.T, mountpoint string) { + t.Helper() if err := RemoveFileBinding(mountpoint); err != nil { t.Logf("failed to remove file binding from %s: %q", mountpoint, err) } } func TestApplyFileBindingReadOnly(t *testing.T) { + requireElevated(t) + source := t.TempDir() destination := t.TempDir() fileName := "testFile.txt" @@ -108,19 +112,14 @@ func TestApplyFileBindingReadOnly(t *testing.T) { } func TestEnsureOnlyOneTargetCanBeMounted(t *testing.T) { - version, err := getWindowsBuildNumber() - if err != nil { - t.Fatalf("couldn't get version number: %s", err) - } + requireElevated(t) + requireBuild(t, RS5+1) // support added after RS5 - if version <= 17763 { - t.Skip("not supported on RS5 or earlier") - } source := t.TempDir() secondarySource := t.TempDir() destination := t.TempDir() - err = ApplyFileBinding(destination, source, false) + err := ApplyFileBinding(destination, source, false) if err != nil { t.Fatal(err) } @@ -159,14 +158,9 @@ func checkSourceIsMountedOnDestination(src, dst string) (bool, error) { } func TestGetBindMappings(t *testing.T) { - version, err := getWindowsBuildNumber() - if err != nil { - t.Fatalf("couldn't get version number: %s", err) - } + requireElevated(t) + requireBuild(t, RS5+1) // support added after RS5 - if version <= 17763 { - t.Skip("not supported on RS5 or earlier") - } // GetBindMappings will expand short paths like ADMINI~1 and PROGRA~1 to their // full names. In order to properly match the names later, we expand them here. srcShort := t.TempDir() @@ -198,6 +192,8 @@ func TestGetBindMappings(t *testing.T) { } func TestRemoveFileBinding(t *testing.T) { + requireElevated(t) + srcShort := t.TempDir() source, err := getFinalPath(srcShort) if err != nil { @@ -238,32 +234,9 @@ func TestRemoveFileBinding(t *testing.T) { } } -func getWindowsBuildNumber() (int, error) { - k, err := registry.OpenKey(registry.LOCAL_MACHINE, `SOFTWARE\Microsoft\Windows NT\CurrentVersion`, registry.QUERY_VALUE) - if err != nil { - return 0, fmt.Errorf("read CurrentVersion reg key: %w", err) - } - defer k.Close() - buildNumStr, _, err := k.GetStringValue("CurrentBuild") - if err != nil { - return 0, fmt.Errorf("read CurrentBuild reg value: %w", err) - } - buildNum, err := strconv.Atoi(buildNumStr) - if err != nil { - return 0, err - } - return buildNum, nil -} - func TestGetBindMappingsSymlinks(t *testing.T) { - version, err := getWindowsBuildNumber() - if err != nil { - t.Fatalf("couldn't get version number: %s", err) - } - - if version <= 17763 { - t.Skip("not supported on RS5 or earlier") - } + requireElevated(t) + requireBuild(t, RS5+1) // support added after RS5 srcShort := t.TempDir() sourceNested := filepath.Join(srcShort, "source") @@ -307,3 +280,23 @@ func TestGetBindMappingsSymlinks(t *testing.T) { t.Fatalf("expected to find %s mounted on %s, but could not", source, destination) } } + +func requireElevated(tb testing.TB) { + tb.Helper() + if !windows.GetCurrentProcessToken().IsElevated() { + tb.Skip("requires elevated privileges") + } +} + +const RS5 = 17763 + +//todo: also check that `bindfltapi.dll` exists + +// require current build to be >= build +func requireBuild(tb testing.TB, build uint32) { + tb.Helper() + _, _, b := windows.RtlGetNtVersionNumbers() + if b < build { + tb.Skipf("requires build %d+; current build is %d", build, b) + } +} diff --git a/pkg/etw/provider_test.go b/pkg/etw/provider_test.go index 1a98a1a..7d5f808 100644 --- a/pkg/etw/provider_test.go +++ b/pkg/etw/provider_test.go @@ -10,6 +10,8 @@ import ( ) func mustGUIDFromString(t *testing.T, s string) guid.GUID { + t.Helper() + g, err := guid.FromString(s) if err != nil { t.Fatal(err) diff --git a/pkg/guid/guid_test.go b/pkg/guid/guid_test.go index 91ac0e6..83c0e23 100644 --- a/pkg/guid/guid_test.go +++ b/pkg/guid/guid_test.go @@ -7,6 +7,8 @@ import ( ) func mustNewV4(t *testing.T) GUID { + t.Helper() + g, err := NewV4() if err != nil { t.Fatal(err) @@ -15,6 +17,8 @@ func mustNewV4(t *testing.T) GUID { } func mustNewV5(t *testing.T, namespace GUID, name []byte) GUID { + t.Helper() + g, err := NewV5(namespace, name) if err != nil { t.Fatal(err) @@ -23,6 +27,8 @@ func mustNewV5(t *testing.T, namespace GUID, name []byte) GUID { } func mustFromString(t *testing.T, s string) GUID { + t.Helper() + g, err := FromString(s) if err != nil { t.Fatal(err) diff --git a/pkg/security/grantvmgroupaccess_test.go b/pkg/security/grantvmgroupaccess_test.go index bd64893..16df644 100644 --- a/pkg/security/grantvmgroupaccess_test.go +++ b/pkg/security/grantvmgroupaccess_test.go @@ -87,6 +87,8 @@ func TestGrantVmGroupAccess(t *testing.T) { } func verifyVMAccountDACLs(t *testing.T, name string, permissions []string) { + t.Helper() + cmd := exec.Command("icacls", name) outb, err := cmd.CombinedOutput() if err != nil {