Merge commit from fork

[28.x] plugin: Fix off-by-one in privilege validation

plugin/manager.go

@@ -1,13 +1,13 @@
 package plugin
 
 import (
+	"cmp"
 	"context"
 	"encoding/json"
 	"io"
 	"os"
 	"path/filepath"
-	"reflect"
-	"sort"
+	"slices"
 	"strings"
 	"sync"
 	"syscall"
@@ -337,34 +337,42 @@ func makeLoggerStreams(id string) (stdout, stderr io.WriteCloser) {
 }
 
 func validatePrivileges(requiredPrivileges, privileges types.PluginPrivileges) error {
-	if !isEqual(requiredPrivileges, privileges, isEqualPrivilege) {
+	if len(requiredPrivileges) != len(privileges) {
 		return errors.New("incorrect privileges")
 	}
 
+	a := normalizePrivileges(requiredPrivileges)
+	b := normalizePrivileges(privileges)
+
+	for i := range a {
+		if a[i].Name != b[i].Name {
+			return errors.New("incorrect privileges")
+		}
+		if !slices.Equal(a[i].Value, b[i].Value) {
+			return errors.New("incorrect privileges")
+		}
+	}
+
 	return nil
 }
 
-func isEqual(arrOne, arrOther types.PluginPrivileges, compare func(x, y types.PluginPrivilege) bool) bool {
-	if len(arrOne) != len(arrOther) {
-		return false
-	}
-
-	sort.Sort(arrOne)
-	sort.Sort(arrOther)
-
-	for i := 1; i < arrOne.Len(); i++ {
-		if !compare(arrOne[i], arrOther[i]) {
-			return false
+// normalizePrivileges returns a normalized copy of privileges with privilege names
+// and each privilege's values sorted for order-insensitive comparison.
+// The input is not mutated.
+func normalizePrivileges(privileges types.PluginPrivileges) types.PluginPrivileges {
+	normalized := make(types.PluginPrivileges, len(privileges))
+	for i, privilege := range privileges {
+		normalized[i] = types.PluginPrivilege{
+			Name:        privilege.Name,
+			Description: privilege.Description,
+			Value:       slices.Clone(privilege.Value),
 		}
+		slices.Sort(normalized[i].Value)
 	}
 
-	return true
-}
+	slices.SortFunc(normalized, func(a, b types.PluginPrivilege) int {
+		return cmp.Compare(a.Name, b.Name)
+	})
 
-func isEqualPrivilege(a, b types.PluginPrivilege) bool {
-	if a.Name != b.Name {
-		return false
-	}
-
-	return reflect.DeepEqual(a.Value, b.Value)
+	return normalized
 }

plugin/manager_test.go

@@ -44,12 +44,48 @@ func TestValidatePrivileges(t *testing.T) {
 			},
 			result: true,
 		},
+		"single-element-same": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"true"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"true"}},
+			},
+			result: true,
+		},
+		"single-element-diff-value": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"false"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"true"}},
+			},
+			result: false,
+		},
+		"first-sorted-element-diff-value": {
+			requiredPrivileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"false"}},
+				{Name: "network", Description: "Description", Value: []string{"host"}},
+			},
+			privileges: []types.PluginPrivilege{
+				{Name: "allow-all-devices", Description: "Description", Value: []string{"true"}},
+				{Name: "network", Description: "Description", Value: []string{"host"}},
+			},
+			result: false,
+		},
+		"empty-privileges": {
+			requiredPrivileges: []types.PluginPrivilege{},
+			privileges:         []types.PluginPrivilege{},
+			result:             true,
+		},
 	}
 
 	for key, data := range testData {
-		err := validatePrivileges(data.requiredPrivileges, data.privileges)
-		if (err == nil) != data.result {
-			t.Fatalf("Test item %s expected result to be %t, got %t", key, data.result, (err == nil))
-		}
+		t.Run(key, func(t *testing.T) {
+			err := validatePrivileges(data.requiredPrivileges, data.privileges)
+			if (err == nil) != data.result {
+				t.Fatalf("expected result to be %t, got %t", data.result, (err == nil))
+			}
+		})
 	}
 }