deps(tools): bump github.com/golangci/golangci-lint from 1.60.1 to 1.…

…60.2 in /tools (#877)

storeapi/go-wrapper/microsoftstore/errors.go

@@ -2,7 +2,7 @@
 package microsoftstore
 
 // StoreAPIError are the error constants in the store api.
-type StoreAPIError int
+type StoreAPIError int64
 
 // Keep up-to-date with `storeapi\base\Exception.hpp`.
 const (
@@ -26,7 +26,7 @@ const (
 )
 
 // NewStoreAPIError creates StoreAPIError from the result of a call to the storeAPI DLL.
-func NewStoreAPIError(hresult uintptr) error {
+func NewStoreAPIError(hresult int64) error {
 	if err := StoreAPIError(hresult); err < ErrSuccess {
 		return err
 	}

storeapi/go-wrapper/microsoftstore/export_test.go

@@ -2,3 +2,6 @@ package microsoftstore
 
 // FindWorkspaceRoot climbs up the current working directory until the Go workspace root is found.
 var FindWorkspaceRoot = findWorkspaceRoot
+
+// CheckError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go.
+var CheckError = checkError

storeapi/go-wrapper/microsoftstore/store.go

@@ -2,8 +2,10 @@ package microsoftstore
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"syscall"
 )
 
 // findWorkspaceRoot climbs up the current working directory until the Go workspace root is found.
@@ -26,3 +28,40 @@ func findWorkspaceRoot() (string, error) {
 		}
 	}
 }
+
+// checkError inspects the values of hres and err to determine what kind of error we have, if any, according to the rules of syscall/dll_windows.go.
+func checkError(hres int64, err error) (int64, error) {
+	// From syscall/dll_windows.go (*Proc).Call doc:
+	// > Callers must inspect the primary return value to decide whether an
+	//   error occurred [...] before consulting the error.
+	// There is no possibility of nil  error, the `err` return value is always constructed with the
+	// result of `GetLastError()` which could have been set by something completely
+	// unrelated to our code some time in the past, as well as it could be `ERROR_SUCCESS` which is the `Errno(0)`.
+	// If the act of calling the API fails (not the function we're calling, but the attempt to call it), then we'd
+	// have a meaningful `syscall.Errno` object via the `err` parameter, related to the actual failure (like a function not found in this DLL)
+	// Since our implementation of the store API doesn't touch errno the call should return `hres`
+	// in our predefined range plus garbage in the `err` argument, thus we only care about the `hres` in this case.
+	if e := NewStoreAPIError(hres); e != nil {
+		return hres, fmt.Errorf("storeApi returned error code %d: %w", hres, e)
+	}
+
+	// Supposedly unreachable: proc.Call must always return a non-nil syscall.Errno
+	if err == nil {
+		return hres, nil
+	}
+
+	var target syscall.Errno
+	if b := errors.As(err, &target); !b {
+		// Supposedly unreachable: proc.Call must always return a non-nil syscall.Errno
+		return hres, err
+	}
+
+	// The act of calling our API didn't succeed, function not found in the DLL for example:
+	if target != syscall.Errno(0) {
+		return hres, fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err)
+	}
+
+	// A non-error value in hres plus ERROR_SUCCESS in err.
+	// This shouldn't happen in the current store API implementation anyway.
+	return hres, nil
+}

storeapi/go-wrapper/microsoftstore/store_test.go

@@ -2,12 +2,14 @@ package microsoftstore_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log/slog"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"syscall"
 	"testing"
 	"time"
 
@@ -87,6 +89,39 @@ func TestGetSubscriptionExpirationDate(t *testing.T) {
 	require.ErrorIs(t, gotErr, wantErr, "GetSubscriptionExpirationDate should have returned code %d", wantErr)
 }
 
+func TestErrorVerification(t *testing.T) {
+	t.Parallel()
+	testcases := map[string]struct {
+		hresult int64
+		err     error
+
+		wantErr bool
+	}{
+		"Success": {},
+		// If HRESULT is not in the Store API error range and err is not a syscall.Errno then we don't have an error.
+		"With an unknown value (not an error)": {hresult: 1, wantErr: false},
+
+		"Upper bound of the Store API enum range": {hresult: -1, wantErr: true},
+		"Lower bound of the Store API enum range": {hresult: int64(microsoftstore.ErrNotSubscribed), wantErr: true},
+		"With a system error (errno)":             {hresult: 32 /*garbage*/, err: syscall.Errno(2) /*E_FILE_NOT_FOUND*/, wantErr: true},
+		"With a generic (unreachable) error":      {hresult: 1, err: errors.New("test error"), wantErr: true},
+		// This would mean an API call returning a non-error hresult plus GetLastError() returning ERROR_SUCCESS
+		// This shouldn't happen in the current store API implementation anyway.
+		"With weird successful error": {hresult: 1, err: syscall.Errno(0) /*ERROR_SUCCESS*/},
+	}
+	for name, tc := range testcases {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			res, err := microsoftstore.CheckError(tc.hresult, tc.err)
+			if tc.wantErr {
+				require.Error(t, err, "CheckError should have returned an error for value: %v, returned value was: %v", tc.hresult, res)
+				return
+			}
+			require.NoError(t, err, "CheckError should have not returned an error for value: %v, returned value was: %v", tc.hresult, res)
+		})
+	}
+}
+
 func buildStoreAPI(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
 	defer cancel()

storeapi/go-wrapper/microsoftstore/store_windows.go

@@ -2,7 +2,6 @@
 package microsoftstore
 
 import (
-	"errors"
 	"fmt"
 	"sync"
 	"syscall"
@@ -81,7 +80,7 @@ func GetSubscriptionExpirationDate() (tm time.Time, err error) {
 // Use this instead of proc.Call to avoid panics.
 //
 //nolint:unparam // Return value is provided to follow convention.
-func call(proc *syscall.LazyProc, args ...uintptr) (int, error) {
+func call(proc *syscall.LazyProc, args ...uintptr) (int64, error) {
 	if err := loadDll(); err != nil {
 		return 0, err
 	}
@@ -92,29 +91,8 @@ func call(proc *syscall.LazyProc, args ...uintptr) (int, error) {
 	}
 
 	hresult, _, err := proc.Call(args...)
-
-	// From syscall/dll_windows.go (*Proc).Call doc:
-	// > Callers must inspect the primary return value to decide whether an
-	//   error occurred [...] before consulting the error.
-	if err := NewStoreAPIError(hresult); err != nil {
-		return int(hresult), fmt.Errorf("storeApi returned error code %d: %w", int(hresult), err)
-	}
-
-	if err == nil {
-		return int(hresult), nil
-	}
-
-	var target syscall.Errno
-	if b := errors.As(err, &target); !b {
-		// Supposedly unrechable: proc.Call must always return a syscall.Errno
-		return int(hresult), err
-	}
-
-	if target != syscall.Errno(0) {
-		return int(hresult), fmt.Errorf("failed syscall to storeApi: %v (syscall errno %d)", target, err)
-	}
-
-	return int(hresult), nil
+	//nolint:gosec // Windows HRESULTS are guaranteed to be 32-bit vlaue, thus they surely fit inside a int64 without overflow.
+	return checkError(int64(hresult), err)
 }
 
 // loadDll finds the dll and ensures it loads.

tools/go.mod

@@ -6,7 +6,7 @@ toolchain go1.22.5
 require (
 	github.com/canonical/ubuntu-pro-for-wsl/windows-agent v0.0.0-20240626165418-426bf0d3695a
 	github.com/canonical/ubuntu-pro-for-wsl/wsl-pro-service v0.0.0-20240705150615-15d23a639880
-	github.com/golangci/golangci-lint v1.60.1
+	github.com/golangci/golangci-lint v1.60.2
 	github.com/spf13/cobra v1.8.1
 	github.com/stretchr/testify v1.9.0
 	golang.org/x/vuln v1.1.3
@@ -85,7 +85,7 @@ require (
 	github.com/gofrs/flock v0.12.1 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/golangci/dupl v0.0.0-20180902072040-3e9179ac440a // indirect
-	github.com/golangci/gofmt v0.0.0-20231018234816-f50ced29576e // indirect
+	github.com/golangci/gofmt v0.0.0-20240816233607-d8596aa466a9 // indirect
 	github.com/golangci/misspell v0.6.0 // indirect
 	github.com/golangci/modinfo v0.3.4 // indirect
 	github.com/golangci/plugin-module-register v0.1.1 // indirect
@@ -156,7 +156,7 @@ require (
 	github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 // indirect
 	github.com/sashamelentyev/interfacebloat v1.1.0 // indirect
 	github.com/sashamelentyev/usestdlibvars v1.27.0 // indirect
-	github.com/securego/gosec/v2 v2.20.1-0.20240525090044-5f0084eb01a9 // indirect
+	github.com/securego/gosec/v2 v2.20.1-0.20240820084340-81cda2f91fbe // indirect
 	github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c // indirect
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/sivchari/containedctx v1.0.3 // indirect
@@ -177,7 +177,7 @@ require (
 	github.com/tetafro/godot v1.4.16 // indirect
 	github.com/timakin/bodyclose v0.0.0-20230421092635-574207250966 // indirect
 	github.com/timonwong/loggercheck v0.9.4 // indirect
-	github.com/tomarrell/wrapcheck/v2 v2.8.3 // indirect
+	github.com/tomarrell/wrapcheck/v2 v2.9.0 // indirect
 	github.com/tommy-muehle/go-mnd/v2 v2.5.1 // indirect
 	github.com/ubuntu/decorate v0.0.0-20240425133904-a085253511fb // indirect
 	github.com/ubuntu/gowsl v0.0.0-20240709100851-bc766d3dbd53 // indirect
@@ -195,7 +195,7 @@ require (
 	go.uber.org/automaxprocs v1.5.3 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	go.uber.org/zap v1.24.0 // indirect
-	golang.org/x/exp v0.0.0-20240707233637-46b078467d37 // indirect
+	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
 	golang.org/x/exp/typeparams v0.0.0-20240314144324-c7f7c6466f7f // indirect
 	golang.org/x/mod v0.20.0 // indirect
 	golang.org/x/net v0.28.0 // indirect
@@ -204,11 +204,11 @@ require (
 	golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7 // indirect
 	golang.org/x/text v0.17.0 // indirect
 	golang.org/x/tools v0.24.0 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20240709173604-40e1e62336c5 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240730163845-b1a4ccb954bf // indirect
 	google.golang.org/grpc v1.65.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
-	honnef.co/go/tools v0.5.0 // indirect
-	mvdan.cc/gofumpt v0.6.0 // indirect
+	honnef.co/go/tools v0.5.1 // indirect
+	mvdan.cc/gofumpt v0.7.0 // indirect
 	mvdan.cc/unparam v0.0.0-20240528143540-8a5130ca722f // indirect
 )