fix(plugins): update VCS plugins from the cached repo, not the install dir

'helm plugin update' ran the dirty check and pull against the installed
plugin directory itself: updatePlugin resolves p.Dir() and FindSource
builds the VCS repo on that path, which Install() made a git checkout by
copying .git from the cached clone. A plugin whose install hook rewrites
files inside its own directory (as helm-unittest's install-binary.sh
does) leaves that checkout permanently dirty, so every subsequent update
fails with "plugin repo was modified".

Point existingVCSRepo at Helm's cached clone of the plugin repository
instead. Hooks only ever run against the installed copy, so the cache
stays pristine: Update() now syncs the cache (recloning it if it was
evicted) and refreshes the installed copy from that clean checkout,
mirroring Install() and OCIInstaller.Update(). The dirty guard is kept
for the cache itself, where it means what it says again.

Closes #31664

Signed-off-by: Bisman-Singh <bismanmadaan1@gmail.com>

internal/plugin/installer/vcs_installer.go

@@ -38,16 +38,18 @@ type VCSInstaller struct {
 	base
 }
 
+// existingVCSRepo reads the remote source of an installed plugin and returns
+// a VCSInstaller operating on Helm's cached copy of that repository. The
+// installed plugin directory is never used as the VCS working tree: a
+// plugin's install/update hooks may legitimately modify files there (for
+// example to place the plugin's binary), which would otherwise leave the
+// checkout permanently dirty and block every future update.
 func existingVCSRepo(location string) (Installer, error) {
 	repo, err := vcs.NewRepo("", location)
 	if err != nil {
 		return nil, err
 	}
-	i := &VCSInstaller{
-		Repo: repo,
-		base: newBase(repo.Remote()),
-	}
-	return i, nil
+	return NewVCSInstaller(repo.Remote(), "")
 }
 
 // NewVCSInstaller creates a new VCSInstaller.
@@ -95,19 +97,26 @@ func (i *VCSInstaller) Install() error {
 	return fs.CopyDir(i.Repo.LocalPath(), i.Path())
 }
 
-// Update updates a remote repository
+// Update fetches the latest version of the plugin repository into Helm's
+// plugin cache and refreshes the installed copy from that clean checkout.
+// The plugin's hooks only ever run against the installed copy, so changes
+// they make are not mistaken for user modifications of the repository.
 func (i *VCSInstaller) Update() error {
 	slog.Debug("updating", "source", i.Repo.Remote())
-	if i.Repo.IsDirty() {
+	if i.Repo.CheckLocal() && i.Repo.IsDirty() {
 		return errors.New("plugin repo was modified")
 	}
-	if err := i.Repo.Update(); err != nil {
+	if err := i.sync(i.Repo); err != nil {
 		return err
 	}
 	if !isPlugin(i.Repo.LocalPath()) {
 		return ErrMissingMetadata
 	}
-	return nil
+	if err := os.RemoveAll(i.Path()); err != nil {
+		return err
+	}
+	slog.Debug("copying files", "source", i.Repo.LocalPath(), "destination", i.Path())
+	return fs.CopyDir(i.Repo.LocalPath(), i.Path())
 }
 
 func (i *VCSInstaller) solveVersion(repo vcs.Repo) (string, error) {

internal/plugin/installer/vcs_installer_test.go

@@ -18,6 +18,7 @@ package installer
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -143,3 +144,61 @@ func TestVCSInstallerUpdate(t *testing.T) {
 	// Testing update for error
 	require.EqualErrorf(t, Update(vcsInstaller), "plugin repo was modified", "expected error for plugin modified")
 }
+
+// runGit runs a git command in dir, failing the test on error.
+func runGit(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	args = append([]string{"-C", dir, "-c", "user.name=Helm Test", "-c", "user.email=helm@example.com", "-c", "commit.gpgsign=false"}, args...)
+	out, err := exec.CommandContext(t.Context(), "git", args...).CombinedOutput()
+	require.NoErrorf(t, err, "git %v: %s", args, out)
+}
+
+// TestVCSInstallerUpdateHookModifiedPlugin ensures a plugin can still be
+// updated after its install hook has modified files inside the installed
+// plugin directory, as helm-unittest's install-binary.sh does. Update must
+// run against Helm's cached repository, not the installed copy, whose
+// checkout is legitimately dirtied by hooks. See
+// https://github.com/helm/helm/issues/31664.
+func TestVCSInstallerUpdateHookModifiedPlugin(t *testing.T) {
+	ensure.HelmHome(t)
+
+	require.NoErrorf(t, os.MkdirAll(helmpath.DataPath("plugins"), 0o755), "Could not create %s", helmpath.DataPath("plugins"))
+
+	// Create a local git repository containing the plugin.
+	upstream := filepath.Join(t.TempDir(), "hookmod")
+	require.NoError(t, os.MkdirAll(upstream, 0o755))
+	pluginYAML := "name: \"hookmod\"\nversion: \"0.1.0\"\ntype: cli/v1\napiVersion: v1\nruntime: subprocess\nconfig:\n  shortHelp: \"hook modified plugin\"\n  longHelp: \"hook modified plugin\"\nruntimeConfig:\n  command: \"echo Hello\"\n"
+	require.NoError(t, os.WriteFile(filepath.Join(upstream, "plugin.yaml"), []byte(pluginYAML), 0o644))
+	runGit(t, upstream, "init")
+	runGit(t, upstream, "add", "plugin.yaml")
+	runGit(t, upstream, "commit", "-m", "initial commit")
+
+	source := "file://" + upstream
+	i, err := NewForSource(source, "")
+	require.NoError(t, err)
+	require.IsType(t, &VCSInstaller{}, i, "expected a VCSInstaller")
+	require.NoError(t, Install(i))
+
+	// Simulate an install hook rewriting a tracked file inside the
+	// installed plugin directory.
+	require.NoError(t, os.WriteFile(filepath.Join(i.Path(), "plugin.yaml"), []byte(pluginYAML+"# rewritten by install hook\n"), 0o644))
+
+	// Publish a new version upstream.
+	require.NoError(t, os.WriteFile(filepath.Join(upstream, "plugin.yaml"), []byte(strings.ReplaceAll(pluginYAML, "0.1.0", "0.2.0")), 0o644))
+	runGit(t, upstream, "commit", "-am", "release 0.2.0")
+
+	upd, err := FindSource(i.Path())
+	require.NoError(t, err)
+	require.NoError(t, Update(upd), "update must not treat hook-produced changes as user modifications")
+
+	// The installed copy is refreshed from the clean checkout.
+	data, err := os.ReadFile(filepath.Join(i.Path(), "plugin.yaml"))
+	require.NoError(t, err)
+	require.Contains(t, string(data), `version: "0.2.0"`)
+	require.NotContains(t, string(data), "rewritten by install hook")
+
+	// A second update must keep working.
+	upd, err = FindSource(i.Path())
+	require.NoError(t, err)
+	require.NoError(t, Update(upd))
+}