fix: harden HELM_PLUGINS list handling and clean up dead code

Guard newBase() in internal/plugin/installer/base.go against an empty
HELM_PLUGINS value: filepath.SplitList('') returns []string{}, so the
previous [0] indexing would panic. Use the raw value as fallback when
the split result is empty.

Remove the now-unused settings field from OCIInstaller: after the prior
fix that switched Path() to use i.base.PluginsDirectory, the field was
never read. Drop it from the struct, NewOCIInstaller, and update the
test accordingly.

Fix two pre-existing test portability problems on Windows:
- http_installer_test.go uses syscall.Umask which does not exist on
  Windows; guard the file with //go:build !windows and move the shared
  mockArchiveServer helper to testhelper_test.go so installer_test.go
  keeps compiling on all platforms.
- TestOCIInstaller_Install_ComponentExtraction checks Unix execute bits
  (mode & 0111) which are always zero on Windows; skip that assertion
  on Windows via runtime.GOOS.

Also make TestPath cross-platform by using filepath.FromSlash and
filepath.Join for expected paths instead of hard-coded Unix strings.

Fixes https://github.com/helm/helm/issues/11310

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Arnav Nagzirkar <113314200+arnavnagzirkar@users.noreply.github.com>
pull/32242/head
Arnav Nagzirkar 4 months ago
parent 25fd5d503a
commit a085bab5a5

@ -32,7 +32,10 @@ func newBase(source string) base {
settings := cli.New() settings := cli.New()
// When HELM_PLUGINS contains a list of paths, use only the first one for // When HELM_PLUGINS contains a list of paths, use only the first one for
// installation so the plugin ends up in a directory that is actually searched. // installation so the plugin ends up in a directory that is actually searched.
pluginsDir := filepath.SplitList(settings.PluginsDirectory)[0] pluginsDir := settings.PluginsDirectory
if dirs := filepath.SplitList(pluginsDir); len(dirs) > 0 {
pluginsDir = dirs[0]
}
return base{ return base{
Source: source, Source: source,
PluginsDirectory: pluginsDir, PluginsDirectory: pluginsDir,

@ -19,6 +19,7 @@ import (
) )
func TestPath(t *testing.T) { func TestPath(t *testing.T) {
pluginsDir := filepath.FromSlash("/helm/data/plugins")
tests := []struct { tests := []struct {
source string source string
helmPluginsDir string helmPluginsDir string
@ -26,12 +27,12 @@ func TestPath(t *testing.T) {
}{ }{
{ {
source: "", source: "",
helmPluginsDir: "/helm/data/plugins", helmPluginsDir: pluginsDir,
expectPath: "", expectPath: "",
}, { }, {
source: "https://github.com/jkroepke/helm-secrets", source: "https://github.com/jkroepke/helm-secrets",
helmPluginsDir: "/helm/data/plugins", helmPluginsDir: pluginsDir,
expectPath: "/helm/data/plugins/helm-secrets", expectPath: filepath.Join(pluginsDir, "helm-secrets"),
}, },
} }
@ -60,3 +61,11 @@ func TestPathMultiplePluginDirs(t *testing.T) {
t.Errorf("expected path %s, got %s", expected, got) t.Errorf("expected path %s, got %s", expected, got)
} }
} }
func TestPathEmptyPluginDir(t *testing.T) {
// When HELM_PLUGINS is explicitly empty, newBase must not panic.
t.Setenv("HELM_PLUGINS", "")
b := newBase("https://github.com/jkroepke/helm-secrets")
// Path() returns "" when source is "" or PluginsDirectory is ""; just verify no panic.
_ = b.Path()
}

@ -13,6 +13,8 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
//go:build !windows
package installer // import "helm.sh/helm/v4/internal/plugin/installer" package installer // import "helm.sh/helm/v4/internal/plugin/installer"
import ( import (
@ -21,13 +23,9 @@ import (
"compress/gzip" "compress/gzip"
"encoding/base64" "encoding/base64"
"errors" "errors"
"fmt"
"io/fs" "io/fs"
"net/http"
"net/http/httptest"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"syscall" "syscall"
"testing" "testing"
@ -66,18 +64,6 @@ func TestStripName(t *testing.T) {
} }
} }
func mockArchiveServer() *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !strings.HasSuffix(r.URL.Path, ".tar.gz") {
w.Header().Add("Content-Type", "text/html")
fmt.Fprintln(w, "broken")
return
}
w.Header().Add("Content-Type", "application/gzip")
fmt.Fprintln(w, "test")
}))
}
func TestHTTPInstaller(t *testing.T) { func TestHTTPInstaller(t *testing.T) {
ensure.HelmHome(t) ensure.HelmHome(t)

@ -29,7 +29,6 @@ import (
"helm.sh/helm/v4/internal/plugin" "helm.sh/helm/v4/internal/plugin"
"helm.sh/helm/v4/internal/plugin/cache" "helm.sh/helm/v4/internal/plugin/cache"
"helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/internal/third_party/dep/fs"
"helm.sh/helm/v4/pkg/cli"
"helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/getter"
"helm.sh/helm/v4/pkg/helmpath" "helm.sh/helm/v4/pkg/helmpath"
"helm.sh/helm/v4/pkg/registry" "helm.sh/helm/v4/pkg/registry"
@ -43,8 +42,7 @@ type OCIInstaller struct {
CacheDir string CacheDir string
PluginName string PluginName string
base base
settings *cli.EnvSettings getter getter.Getter
getter getter.Getter
// Cached data to avoid duplicate downloads // Cached data to avoid duplicate downloads
pluginData []byte pluginData []byte
provData []byte provData []byte
@ -63,8 +61,6 @@ func NewOCIInstaller(source string, options ...getter.Option) (*OCIInstaller, er
return nil, err return nil, err
} }
settings := cli.New()
// Always add plugin artifact type and any provided options // Always add plugin artifact type and any provided options
pluginOptions := append([]getter.Option{getter.WithArtifactType("plugin")}, options...) pluginOptions := append([]getter.Option{getter.WithArtifactType("plugin")}, options...)
getterProvider, err := getter.NewOCIGetter(pluginOptions...) getterProvider, err := getter.NewOCIGetter(pluginOptions...)
@ -76,7 +72,6 @@ func NewOCIInstaller(source string, options ...getter.Option) (*OCIInstaller, er
CacheDir: helmpath.CachePath("plugins", key), CacheDir: helmpath.CachePath("plugins", key),
PluginName: pluginName, PluginName: pluginName,
base: newBase(source), base: newBase(source),
settings: settings,
getter: getterProvider, getter: getterProvider,
} }
return i, nil return i, nil

@ -27,6 +27,7 @@ import (
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
"runtime"
"strings" "strings"
"testing" "testing"
"time" "time"
@ -35,7 +36,6 @@ import (
ocispec "github.com/opencontainers/image-spec/specs-go/v1" ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/internal/test/ensure"
"helm.sh/helm/v4/pkg/cli"
"helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/getter"
"helm.sh/helm/v4/pkg/helmpath" "helm.sh/helm/v4/pkg/helmpath"
) )
@ -266,10 +266,6 @@ func TestNewOCIInstaller(t *testing.T) {
t.Errorf("expected cache directory to contain 'plugins', got %s", installer.CacheDir) t.Errorf("expected cache directory to contain 'plugins', got %s", installer.CacheDir)
} }
if installer.settings == nil {
t.Error("expected settings to be initialized")
}
// Check that Path() method works // Check that Path() method works
expectedPath := helmpath.DataPath("plugins", tt.expectName) expectedPath := helmpath.DataPath("plugins", tt.expectName)
if installer.Path() != expectedPath { if installer.Path() != expectedPath {
@ -305,7 +301,6 @@ func TestOCIInstaller_Path(t *testing.T) {
installer := &OCIInstaller{ installer := &OCIInstaller{
PluginName: tt.pluginName, PluginName: tt.pluginName,
base: newBase(tt.source), base: newBase(tt.source),
settings: cli.New(),
} }
path := installer.Path() path := installer.Path()
@ -539,7 +534,7 @@ func TestOCIInstaller_Install_ComponentExtraction(t *testing.T) {
execPath := filepath.Join(tempDir, "bin", pluginName) execPath := filepath.Join(tempDir, "bin", pluginName)
if info, err := os.Stat(execPath); err != nil { if info, err := os.Stat(execPath); err != nil {
t.Errorf("executable not found: %v", err) t.Errorf("executable not found: %v", err)
} else if info.Mode()&0111 == 0 { } else if runtime.GOOS != "windows" && info.Mode()&0111 == 0 {
t.Error("file is not executable") t.Error("file is not executable")
} }

@ -0,0 +1,35 @@
/*
Copyright The Helm Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package installer // import "helm.sh/helm/v4/internal/plugin/installer"
import (
"fmt"
"net/http"
"net/http/httptest"
"strings"
)
func mockArchiveServer() *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !strings.HasSuffix(r.URL.Path, ".tar.gz") {
w.Header().Add("Content-Type", "text/html")
fmt.Fprintln(w, "broken")
return
}
w.Header().Add("Content-Type", "application/gzip")
fmt.Fprintln(w, "test")
}))
}
Loading…
Cancel
Save