Merge 280fbb5506 into 70dc9a0027

pkg/plugins/manager/installer.go

@@ -142,7 +142,6 @@ func (m *PluginInstaller) install(ctx context.Context, pluginID, version string,
 	if version != "" && extractedArchive.Version != version {
 		m.log.Error("Installed plugin version mismatch", "expected", version, "got", extractedArchive.Version)
 	}
-
 	return extractedArchive, nil
 }
 

pkg/plugins/storage/fs.go

@@ -43,6 +43,12 @@ func (fs *FS) Extract(ctx context.Context, pluginID string, dirNameFunc DirNameG
 		return nil, fmt.Errorf("%v: %w", "failed to extract plugin archive", err)
 	}
 
+	// Ensure installed plugin directory inherits ownership from parent plugin dir.
+	// Do not fail extraction on error; just warn.
+	if err := matchOwnershipToParent(pluginDir, fs.pluginsDir); err != nil {
+		fs.log.Warn("Failed to set plugin ownership", "path", pluginDir, "err", err)
+	}
+
 	pluginJSON, err := readPluginJSON(pluginDir)
 	if err != nil {
 		return nil, fmt.Errorf("%v: %w", "failed to convert to plugin DTO", err)

pkg/plugins/storage/ownership_nop.go

@@ -0,0 +1,9 @@
+//go:build windows
+// +build windows
+
+package storage
+
+// no-op for Windows
+func matchOwnershipToParent(_, _ string) error {
+	return nil
+}

pkg/plugins/storage/ownership_nop_test.go

@@ -0,0 +1,13 @@
+//go:build windows
+// +build windows
+
+package storage
+
+import "testing"
+
+// On Windows this is a no-op; must return nil.
+func TestMatchOwnershipToParent_NoopOnWindows(t *testing.T) {
+	if err := matchOwnershipToParent("somechild", "someparent"); err != nil {
+		t.Fatalf("expected nil from noop matchOwnershipToParent on Windows, got: %v", err)
+	}
+}

pkg/plugins/storage/ownership_unix.go

@@ -0,0 +1,52 @@
+//go:build !windows
+
+package storage
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// chownFunc is a function type for changing file ownership
+type chownFunc func(name string, uid, gid int) error
+
+// ownershipChanger handles changing file ownership
+type ownershipChanger struct {
+	chown chownFunc
+}
+
+// defaultOwnershipChanger uses the real os.Chown function
+var defaultOwnershipChanger = &ownershipChanger{chown: os.Chown}
+
+// matchOwnershipToParent ensures child directory and all its contents inherit ownership from parent dir (UID/GID).
+func matchOwnershipToParent(child, parent string) error {
+	return matchOwnershipToParentWithChanger(child, parent, defaultOwnershipChanger)
+}
+
+// matchOwnershipToParentWithChanger is the testable version that accepts a custom ownership changer
+func matchOwnershipToParentWithChanger(child, parent string, changer *ownershipChanger) error {
+	parentInfo, err := os.Stat(parent)
+	if err != nil {
+		return err
+	}
+
+	sys := parentInfo.Sys()
+	stat, ok := sys.(*syscall.Stat_t)
+	if !ok {
+		// avoid panics on non-Unix or unexpected platforms
+		return fmt.Errorf("ownership: unsupported stat type %T", sys)
+	}
+
+	uid := int(stat.Uid)
+	gid := int(stat.Gid)
+
+	// Recursively apply ownership to all files and directories
+	return filepath.WalkDir(child, func(path string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		return changer.chown(path, uid, gid)
+	})
+}

pkg/plugins/storage/ownership_unix_test.go

@@ -0,0 +1,140 @@
+//go:build !windows
+
+package storage
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestMatchOwnershipToParent(t *testing.T) {
+	t.Run("Parent dir does not exist", func(t *testing.T) {
+		childDir := t.TempDir()
+		err := matchOwnershipToParent(childDir, filepath.Join(childDir, "no-such-parent"))
+		require.ErrorIs(t, err, os.ErrNotExist)
+	})
+
+	t.Run("Child directory does not exist", func(t *testing.T) {
+		parent := t.TempDir()
+		nonExistentChild := filepath.Join(parent, "does-not-exist")
+		err := matchOwnershipToParent(nonExistentChild, parent)
+		require.ErrorIs(t, err, os.ErrNotExist)
+	})
+
+	t.Run("Should call chown for all files and directories", func(t *testing.T) {
+		parent := t.TempDir()
+		parentUID, parentGID := getOwnership(t, parent)
+
+		// Create test structure
+		child := filepath.Join(parent, "child")
+		err := os.MkdirAll(child, 0o750)
+		require.NoError(t, err)
+
+		nestedDir := filepath.Join(child, "nested")
+		err = os.MkdirAll(nestedDir, 0o750)
+		require.NoError(t, err)
+
+		file1 := filepath.Join(child, "file1.txt")
+		err = os.WriteFile(file1, []byte("test"), 0o644)
+		require.NoError(t, err)
+
+		file2 := filepath.Join(nestedDir, "file2.txt")
+		err = os.WriteFile(file2, []byte("test"), 0o644)
+		require.NoError(t, err)
+
+		mock := &mockChowner{}
+		err = matchOwnershipToParentWithChanger(child, parent, &ownershipChanger{chown: mock.chown})
+		require.NoError(t, err)
+
+		// Verify chown was called for all paths with correct UID/GID
+		require.Len(t, mock.calls, 4)
+
+		expectedPaths := []string{child, file1, nestedDir, file2}
+		actualPaths := make([]string, len(mock.calls))
+		for i, call := range mock.calls {
+			actualPaths[i] = call.path
+			require.Equal(t, parentUID, call.uid)
+			require.Equal(t, parentGID, call.gid)
+		}
+
+		require.ElementsMatch(t, expectedPaths, actualPaths)
+	})
+
+	t.Run("Should handle chown errors", func(t *testing.T) {
+		parent := t.TempDir()
+
+		child := filepath.Join(parent, "child")
+		err := os.MkdirAll(child, 0o750)
+		require.NoError(t, err)
+
+		// Mock chown to return an error
+		chownErr := errors.New("permission denied")
+		mock := &mockChowner{err: chownErr}
+		err = matchOwnershipToParentWithChanger(child, parent, &ownershipChanger{chown: mock.chown})
+		require.ErrorIs(t, err, chownErr)
+	})
+
+	t.Run("Should handle single file", func(t *testing.T) {
+		parent := t.TempDir()
+		parentUID, parentGID := getOwnership(t, parent)
+
+		singleFile := filepath.Join(parent, "single-file.txt")
+		err := os.WriteFile(singleFile, []byte("content"), 0o644)
+		require.NoError(t, err)
+
+		mock := &mockChowner{}
+		err = matchOwnershipToParentWithChanger(singleFile, parent, &ownershipChanger{chown: mock.chown})
+		require.NoError(t, err)
+
+		// Should call chown once for the single file
+		require.Len(t, mock.calls, 1)
+		require.Equal(t, singleFile, mock.calls[0].path)
+		require.Equal(t, parentUID, mock.calls[0].uid)
+		require.Equal(t, parentGID, mock.calls[0].gid)
+	})
+
+	t.Run("Should not error with non mock", func(t *testing.T) {
+		parent := t.TempDir()
+
+		singleFile := filepath.Join(parent, "single-file.txt")
+		err := os.WriteFile(singleFile, []byte("content"), 0o644)
+		require.NoError(t, err)
+
+		err = matchOwnershipToParent(singleFile, parent)
+		require.NoError(t, err)
+	})
+}
+
+// getOwnership returns the UID and GID of a file or directory
+func getOwnership(t *testing.T, path string) (uid, gid int) {
+	t.Helper()
+	info, err := os.Stat(path)
+	require.NoError(t, err)
+
+	stat, ok := info.Sys().(*syscall.Stat_t)
+	require.True(t, ok)
+
+	return int(stat.Uid), int(stat.Gid)
+}
+
+// mockChowner records calls to chown for testing
+type mockChowner struct {
+	calls []chownCall
+	err   error // error to return from chown calls
+}
+
+type chownCall struct {
+	path string
+	uid  int
+	gid  int
+}
+
+func (m *mockChowner) chown(path string, uid, gid int) error {
+	m.calls = append(m.calls, chownCall{path: path, uid: uid, gid: gid})
+	return m.err
+}