Merge pull request moby#48824 from thaJeztah/update_golangci_config

fix, and update golangci-lint config, and fix some linting issues

.golangci.yml

@@ -19,19 +19,44 @@ linters:
   disable:
     - errcheck
 
-  run:
-    concurrency: 2
-    modules-download-mode: vendor
-
-    skip-dirs:
-      - docs
+run:
+  # prevent golangci-lint from deducting the go version to lint for through go.mod,
+  # which causes it to fallback to go1.17 semantics.
+  go: "1.23.2"
+  concurrency: 2
+  # Only supported with go modules enabled (build flag -mod=vendor only valid when using modules)
+  # modules-download-mode: vendor
 
 linters-settings:
+  depguard:
+    rules:
+      main:
+        deny:
+          - pkg: io/ioutil
+            desc: The io/ioutil package has been deprecated, see https://go.dev/doc/go1.16#ioutil
+          - pkg: "github.com/stretchr/testify/assert"
+            desc: Use "gotest.tools/v3/assert" instead
+          - pkg: "github.com/stretchr/testify/require"
+            desc: Use "gotest.tools/v3/assert" instead
+          - pkg: "github.com/stretchr/testify/suite"
+            desc: Do not use
+          - pkg: "github.com/containerd/containerd/errdefs"
+            desc: The errdefs package has moved to a separate module, https://github.com/containerd/errdefs
+          - pkg: "github.com/containerd/containerd/log"
+            desc: The logs package has moved to a separate module, https://github.com/containerd/log
+          - pkg: "github.com/containerd/containerd/pkg/userns"
+            desc: Use github.com/moby/sys/userns instead.
+          - pkg: "github.com/opencontainers/runc/libcontainer/userns"
+            desc: Use github.com/moby/sys/userns instead.
+          - pkg: "github.com/tonistiigi/fsutil"
+            desc: The fsutil module does not have a stable API, so we should not have a direct dependency unless necessary.
+
   dupword:
     ignore:
       - "true"    # some tests use this as expected output
       - "false"   # some tests use this as expected output
       - "root"    # for tests using "ls" output with files owned by "root:root"
+
   forbidigo:
     forbid:
       - pkg: ^sync/atomic$
@@ -44,6 +69,20 @@ linters-settings:
         p: ^nlwrap.Handle.(BridgeVlanList|ChainList|ClassList|ConntrackDeleteFilter$|DevLinkGetDeviceList|DevLinkGetAllPortList|DevlinkGetDeviceParams|FilterList|FouList|GenlFamilyList|GTPPDPList|LinkByAlias|LinkSubscribeWithOptions|NeighList$|NeighProxyList|NeighListExecute|NeighSubscribeWithOptions|LinkGetProtinfo|QdiscList|RdmaLinkList|RdmaLinkByName|RdmaLinkDel|RouteListFilteredIter|RuleListFiltered$|RouteSubscribeWithOptions|RuleList$|RuleListFiltered|SocketGet|SocketDiagTCPInfo|SocketDiagTCP|SocketDiagUDPInfo|SocketDiagUDP|UnixSocketDiagInfo|UnixSocketDiag|VDPAGetDevConfigList|VDPAGetDevList|VDPAGetMGMTDevList)
         msg: Add a wrapper to nlwrap.Handle for EINTR handling and update the list in .golangci.yml.
     analyze-types: true
+
+  gosec:
+    excludes:
+      - G104 # G104: Errors unhandled; (TODO: reduce unhandled errors, or explicitly ignore)
+      - G113 # G113: Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772); (only affects go < 1.16.14. and go < 1.17.7)
+      - G115 # G115: integer overflow conversion; (TODO: verify these: https://github.com/moby/moby/issues/48358)
+      - G204 # G204: Subprocess launched with variable; too many false positives.
+      - G301 # G301: Expect directory permissions to be 0750 or less (also EXC0009); too restrictive
+      - G302 # G302: Expect file permissions to be 0600 or less (also EXC0009); too restrictive
+      - G304 # G304: Potential file inclusion via variable.
+      - G306 # G306: Expect WriteFile permissions to be 0600 or less (too restrictive; also flags "0o644" permissions)
+      - G307 # G307: Deferring unsafe method "*os.File" on type "Close" (also EXC0008); (TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close")
+      - G504 # G504: Blocklisted import net/http/cgi: Go versions < 1.6.3 are vulnerable to Httpoxy attack: (CVE-2016-5386); (only affects go < 1.6.3)
+
   importas:
     # Do not allow unaliased imports of aliased packages.
     no-unaliased: true
@@ -56,44 +95,19 @@ linters-settings:
       - pkg: github.com/opencontainers/image-spec/specs-go/v1
         alias: ocispec
 
-  govet:
-    check-shadowing: false
-
-  gosec:
-    excludes:
-      - G115 # FIXME temporarily suppress 'G115: integer overflow conversion': it produces many hits, some of which may be false positives, and need to be looked at; see https://github.com/moby/moby/issues/48358
-
-  depguard:
-    rules:
-      main:
-        deny:
-          - pkg: io/ioutil
-            desc: The io/ioutil package has been deprecated, see https://go.dev/doc/go1.16#ioutil
-          - pkg: "github.com/stretchr/testify/assert"
-            desc: Use "gotest.tools/v3/assert" instead
-          - pkg: "github.com/stretchr/testify/require"
-            desc: Use "gotest.tools/v3/assert" instead
-          - pkg: "github.com/stretchr/testify/suite"
-            desc: Do not use
-          - pkg: "github.com/containerd/containerd/errdefs"
-            desc: The errdefs package has moved to a separate module, https://github.com/containerd/errdefs
-          - pkg: "github.com/containerd/containerd/log"
-            desc: The logs package has moved to a separate module, https://github.com/containerd/log
-          - pkg: "github.com/containerd/containerd/pkg/userns"
-            desc: Use github.com/moby/sys/userns instead.
-          - pkg: "github.com/opencontainers/runc/libcontainer/userns"
-            desc: Use github.com/moby/sys/userns instead.
-          - pkg: "github.com/tonistiigi/fsutil"
-            desc: The fsutil module does not have a stable API, so we should not have a direct dependency unless necessary.
   revive:
     rules:
       # FIXME make sure all packages have a description. Currently, there's many packages without.
       - name: package-comments
         disabled: true
+
 issues:
   # The default exclusion rules are a bit too permissive, so copying the relevant ones below
   exclude-use-default: false
 
+  exclude-dirs:
+    - docs
+
   exclude-rules:
     # We prefer to use an "exclude-list" so that new "default" exclusions are not
     # automatically inherited. We can decide whether or not to follow upstream
@@ -103,44 +117,15 @@ issues:
     # ID's.
     #
     # These exclusion patterns are copied from the default excludes at:
-    # https://github.com/golangci/golangci-lint/blob/v1.46.2/pkg/config/issues.go#L10-L104
+    # https://github.com/golangci/golangci-lint/blob/v1.61.0/pkg/config/issues.go#L11-L104
+    #
+    # The default list of exclusions can be found at:
+    # https://golangci-lint.run/usage/false-positives/#default-exclusions
 
     # EXC0001
     - text: "Error return value of .((os\\.)?std(out|err)\\..*|.*Close|.*Flush|os\\.Remove(All)?|.*print(f|ln)?|os\\.(Un)?Setenv). is not checked"
       linters:
         - errcheck
-    # EXC0006
-    - text: "Use of unsafe calls should be audited"
-      linters:
-        - gosec
-    # EXC0007
-    - text: "Subprocess launch(ed with variable|ing should be audited)"
-      linters:
-        - gosec
-    # EXC0008
-    # TODO: evaluate these and fix where needed: G307: Deferring unsafe method "*os.File" on type "Close" (gosec)
-    - text: "(G104|G307)"
-      linters:
-        - gosec
-    # EXC0009
-    - text: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)"
-      linters:
-        - gosec
-    # EXC0010
-    - text: "Potential file inclusion via variable"
-      linters:
-        - gosec
-
-    # Looks like the match in "EXC0007" above doesn't catch this one
-    # TODO: consider upstreaming this to golangci-lint's default exclusion rules
-    - text: "G204: Subprocess launched with a potential tainted input or cmd arguments"
-      linters:
-        - gosec
-    # Looks like the match in "EXC0009" above doesn't catch this one
-    # TODO: consider upstreaming this to golangci-lint's default exclusion rules
-    - text: "G306: Expect WriteFile permissions to be 0600 or less"
-      linters:
-        - gosec
 
     # Exclude some linters from running on tests files.
     - path: _test\.go

integration/image/remove_unix_test.go

@@ -84,15 +84,15 @@ func TestRemoveImageGarbageCollector(t *testing.T) {
 	file, _ := os.Open(data["UpperDir"])
 	attr := 0x00000010
 	fsflags := uintptr(0x40086602)
-	argp := uintptr(unsafe.Pointer(&attr))
+	argp := uintptr(unsafe.Pointer(&attr)) // #nosec G103 -- Ignore "G103: Use of unsafe calls should be audited"
 	_, _, errno := syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp)
 	assert.Equal(t, "errno 0", errno.Error())
 
 	// Try to remove the image, it should generate error
 	// but marking layer back to mutable before checking errors (so we don't break CI server)
 	_, err = client.ImageRemove(ctx, imgName, image.RemoveOptions{})
 	attr = 0x00000000
-	argp = uintptr(unsafe.Pointer(&attr))
+	argp = uintptr(unsafe.Pointer(&attr)) // #nosec G103 -- Ignore "G103: Use of unsafe calls should be audited"
 	_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp)
 	assert.Equal(t, "errno 0", errno.Error())
 	assert.Assert(t, err != nil)

libnetwork/drivers/bridge/port_mapping_linux.go

@@ -679,7 +679,7 @@ func bindSCTP(cfg portBindingReq, port int) (_ portBinding, retErr error) {
 		uintptr(sd),
 		sctp.SOL_SCTP,
 		sctp.SCTP_INITMSG,
-		uintptr(unsafe.Pointer(&options)),
+		uintptr(unsafe.Pointer(&options)), // #nosec G103 -- Ignore "G103: Use of unsafe calls should be audited"
 		unsafe.Sizeof(options),
 		0); errno != 0 {
 		return portBinding{}, errno

pkg/archive/changes_linux.go

@@ -261,13 +261,13 @@ func readdirnames(dirname string) (names []nameIno, err error) {
 func parseDirent(buf []byte, names []nameIno) (consumed int, newnames []nameIno) {
 	origlen := len(buf)
 	for len(buf) > 0 {
-		dirent := (*unix.Dirent)(unsafe.Pointer(&buf[0]))
+		dirent := (*unix.Dirent)(unsafe.Pointer(&buf[0])) // #nosec G103 -- Ignore "G103: Use of unsafe calls should be audited"
 		buf = buf[dirent.Reclen:]
 		if dirent.Ino == 0 { // File absent in directory.
 			continue
 		}
-		bytes := (*[10000]byte)(unsafe.Pointer(&dirent.Name[0]))
-		name := string(bytes[0:clen(bytes[:])])
+		b := (*[10000]byte)(unsafe.Pointer(&dirent.Name[0])) // #nosec G103 -- Ignore "G103: Use of unsafe calls should be audited"
+		name := string(b[0:clen(b[:])])
 		if name == "." || name == ".." { // Useless names
 			continue
 		}