From 7a906196ede31f747344871c6df8dc7ddb1de61e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 Oct 2024 15:30:46 +0100 Subject: [PATCH 1/5] golangci-lint: enable G204, add #nosec comments instead There's only 3 locations where it's hit, so putting #gosec ignore comments in those locations. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 020f3a7ad961650e312db4c013b549c5cc45e93b) Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 5 ----- cli-plugins/manager/candidate.go | 2 +- cli-plugins/manager/manager.go | 3 ++- cli-plugins/manager/plugin.go | 2 +- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a00093c80d93..920bdd396583 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -146,11 +146,6 @@ issues: - text: "G104" 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" diff --git a/cli-plugins/manager/candidate.go b/cli-plugins/manager/candidate.go index 83e5a05256b1..e65ac1a54f37 100644 --- a/cli-plugins/manager/candidate.go +++ b/cli-plugins/manager/candidate.go @@ -17,5 +17,5 @@ func (c *candidate) Path() string { } func (c *candidate) Metadata() ([]byte, error) { - return exec.Command(c.path, MetadataSubcommandName).Output() + return exec.Command(c.path, MetadataSubcommandName).Output() // #nosec G204 -- ignore "Subprocess launched with a potential tainted input or cmd arguments" } diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go index 9886830240bd..f9229c52576c 100644 --- a/cli-plugins/manager/manager.go +++ b/cli-plugins/manager/manager.go @@ -240,7 +240,8 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command // TODO: why are we not returning plugin.Err? return nil, errPluginNotFound(name) } - cmd := exec.Command(plugin.Path, args...) + cmd := exec.Command(plugin.Path, args...) // #nosec G204 -- ignore "Subprocess launched with a potential tainted input or cmd arguments" + // Using dockerCli.{In,Out,Err}() here results in a hang until something is input. // See: - https://github.com/golang/go/issues/10338 // - https://github.com/golang/go/commit/d000e8742a173aa0659584aa01b7ba2834ba28ab diff --git a/cli-plugins/manager/plugin.go b/cli-plugins/manager/plugin.go index 877241e0b828..5576ef4301ca 100644 --- a/cli-plugins/manager/plugin.go +++ b/cli-plugins/manager/plugin.go @@ -112,7 +112,7 @@ func (p *Plugin) RunHook(ctx context.Context, hookData HookPluginData) ([]byte, return nil, wrapAsPluginError(err, "failed to marshall hook data") } - pCmd := exec.CommandContext(ctx, p.Path, p.Name, HookSubcommandName, string(hDataBytes)) + pCmd := exec.CommandContext(ctx, p.Path, p.Name, HookSubcommandName, string(hDataBytes)) // #nosec G204 -- ignore "Subprocess launched with a potential tainted input or cmd arguments" pCmd.Env = os.Environ() pCmd.Env = append(pCmd.Env, ReexecEnvvar+"="+os.Args[0]) hookCmdOutput, err := pCmd.Output() From 1890ea0762c5704762b430ca9fca41533fd71457 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 Oct 2024 15:34:28 +0100 Subject: [PATCH 2/5] golangci-lint: move gosec excludes to linters-settings Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 7451339ab05561b427f6f473ad2cc908feb95359) Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 920bdd396583..24e3cfca0447 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -54,6 +54,12 @@ linters-settings: desc: The io/ioutil package has been deprecated, see https://go.dev/doc/go1.16#ioutil gocyclo: min-complexity: 16 + 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) + - 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") govet: enable: - shadow @@ -123,11 +129,6 @@ issues: - 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: "G307" - linters: - - gosec # EXC0009 - text: "(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)" linters: @@ -137,21 +138,6 @@ issues: linters: - gosec - # G113 Potential uncontrolled memory consumption in Rat.SetString (CVE-2022-23772) - # only affects gp < 1.16.14. and go < 1.17.7 - - text: "G113" - linters: - - gosec - # TODO: G104: Errors unhandled. (gosec) - - text: "G104" - 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 - # TODO: make sure all packages have a description. Currently, there's 67 packages without. - text: "package-comments: should have a package comment" linters: From 35122a0692c89c1b14630aa6ceea13326fa5d02a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 Oct 2024 15:50:54 +0100 Subject: [PATCH 3/5] golangci-lint: update comment, and disable "exclude-dirs-use-default" Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 745629bd55221dc9dec17ce85f3fd899ff6014dc) Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 24e3cfca0447..48ef790f9322 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -95,6 +95,10 @@ issues: # The default exclusion rules are a bit too permissive, so copying the relevant ones below exclude-use-default: false + # This option has been defined when Go modules was not existed and when the + # golangci-lint core was different, this is not something we still recommend. + exclude-dirs-use-default: false + exclude: - parameter .* always receives @@ -112,6 +116,9 @@ issues: # # These exclusion patterns are copied from the default excluses at: # https://github.com/golangci/golangci-lint/blob/v1.44.0/pkg/config/issues.go#L10-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" From df7113d7ebeec4c3d2b7c41b1ad7f78d945fd9e9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 14 Nov 2024 09:42:49 +0100 Subject: [PATCH 4/5] [27.x] cli: fix non-constant format string in call to errors.Errorf (govet) cli/required.go:17:24: printf: non-constant format string in call to github.com/docker/cli/vendor/github.com/pkg/errors.Errorf (govet) return errors.Errorf("\n" + strings.TrimRight(cmd.UsageString(), "\n")) ^ Signed-off-by: Sebastiaan van Stijn --- cli/required.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/required.go b/cli/required.go index bad16248565c..e8edcaafac2b 100644 --- a/cli/required.go +++ b/cli/required.go @@ -14,7 +14,7 @@ func NoArgs(cmd *cobra.Command, args []string) error { } if cmd.HasSubCommands() { - return errors.Errorf("\n" + strings.TrimRight(cmd.UsageString(), "\n")) + return errors.New("\n" + strings.TrimRight(cmd.UsageString(), "\n")) } return errors.Errorf( From 8fe93724a3563c95444672fdc50c624910ac084a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 Oct 2024 14:27:54 +0100 Subject: [PATCH 5/5] bump golangci-lint to v1.61.0 Also updating a linter that was deprecated; The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 9af049c61810ef2fd997e2f717e21bd3698849ca) Signed-off-by: Sebastiaan van Stijn --- .golangci.yml | 3 ++- dockerfiles/Dockerfile.lint | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 48ef790f9322..4fab014d4758 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,12 +1,12 @@ linters: enable: - bodyclose + - copyloopvar # Detects places where loop variables are copied. - depguard - dogsled - dupword # Detects duplicate words. - durationcheck - errchkjson - - exportloopref # Detects pointers to enclosing loop variables. - gocritic # Metalinter; detects bugs, performance, and styling issues. - gocyclo - gofumpt # Detects whether code was gofumpt-ed. @@ -58,6 +58,7 @@ linters-settings: 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/docker/cli/issues/5584) - 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") govet: diff --git a/dockerfiles/Dockerfile.lint b/dockerfiles/Dockerfile.lint index 05d9fca5769f..00a851ec231d 100644 --- a/dockerfiles/Dockerfile.lint +++ b/dockerfiles/Dockerfile.lint @@ -2,7 +2,7 @@ ARG GO_VERSION=1.22.9 ARG ALPINE_VERSION=3.20 -ARG GOLANGCI_LINT_VERSION=v1.59.1 +ARG GOLANGCI_LINT_VERSION=v1.61.0 FROM golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine AS golangci-lint