MM-68830: Preserve unknown permissions during migrations on downgrade (#36888) (#37005)

Automatic Merge

server/channels/app/permissions_migrations.go

@@ -237,7 +237,10 @@ func (s *Server) doPermissionsMigration(key string, migrationMap permissionsMap,
 
 	for _, role := range roles {
 		role.Permissions = applyPermissionsMap(role, roleMap, migrationMap)
-		if _, err := s.Store().Role().Save(role); err != nil {
+		// Use SavePreservingUnknownPermissions so a server that was downgraded from a
+		// newer release (which wrote permissions this binary doesn't recognize) does
+		// not fail fatally here. Unknown permissions are logged and preserved (MM-68830).
+		if _, err := s.Store().Role().SavePreservingUnknownPermissions(role); err != nil {
 			var invErr *store.ErrInvalidInput
 			switch {
 			case errors.As(err, &invErr):

server/channels/app/permissions_migrations_test.go

@@ -4,6 +4,7 @@
 package app
 
 import (
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -48,7 +49,7 @@ func TestRestoreManageOAuthPermissionMigration(t *testing.T) {
 		return system.Name == model.MigrationKeyRestoreManageOAuthPermission && system.Value == "true"
 	})).Return(nil).Once()
 
-	roleStore.On("Save", mock.AnythingOfType("*model.Role")).
+	roleStore.On("SavePreservingUnknownPermissions", mock.AnythingOfType("*model.Role")).
 		Return(func(role *model.Role) *model.Role { return role }, nil).Twice()
 
 	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyRestoreManageOAuthPermission, migrationMap, roles)
@@ -61,7 +62,7 @@ func TestRestoreManageOAuthPermissionMigration(t *testing.T) {
 	require.Nil(t, appErr)
 	assert.Len(t, systemAdminRole.Permissions, 2)
 
-	roleStore.AssertNumberOfCalls(t, "Save", 2)
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 2)
 	systemStore.AssertNumberOfCalls(t, "SaveOrUpdate", 1)
 }
 
@@ -98,7 +99,7 @@ func TestAddManageAgentPermissionsMigration(t *testing.T) {
 		return system.Name == model.MigrationKeyAddManageAgentPermissions && system.Value == "true"
 	})).Return(nil).Once()
 
-	roleStore.On("Save", mock.AnythingOfType("*model.Role")).
+	roleStore.On("SavePreservingUnknownPermissions", mock.AnythingOfType("*model.Role")).
 		Return(func(role *model.Role) *model.Role { return role }, nil).Twice()
 
 	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyAddManageAgentPermissions, migrationMap, roles)
@@ -115,10 +116,52 @@ func TestAddManageAgentPermissionsMigration(t *testing.T) {
 	assert.Len(t, systemAdminRole.Permissions, 3, "system_admin should still have 3 permissions after idempotent run")
 	assert.Len(t, systemUserRole.Permissions, 2, "system_user should still have 2 permissions after idempotent run")
 
-	roleStore.AssertNumberOfCalls(t, "Save", 2)
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 2)
 	systemStore.AssertNumberOfCalls(t, "SaveOrUpdate", 1)
 }
 
+// TestPermissionsMigrationPreservesUnknownPermissions is the regression test for
+// MM-68830: a server downgraded from a newer release holds permissions the older
+// binary does not recognize. The permissions migration must not fail fatally, and
+// must preserve those unknown permissions rather than stripping them.
+func TestPermissionsMigrationPreservesUnknownPermissions(t *testing.T) {
+	mainHelper.Parallel(t)
+
+	th := SetupWithStoreMock(t)
+
+	migrationMap, err := th.App.getAddManageAgentPermissionsMigration()
+	require.NoError(t, err)
+
+	const unknownPermission = "manage_own_agent_from_the_future"
+	systemAdminRole := &model.Role{
+		Name:        model.SystemAdminRoleId,
+		Permissions: []string{model.PermissionManageSystem.Id, unknownPermission},
+	}
+	roles := []*model.Role{systemAdminRole}
+
+	mockStore := th.App.Srv().Store().(*mocks.Store)
+	roleStore := mocks.RoleStore{}
+	systemStore := mocks.SystemStore{}
+
+	mockStore.On("Role").Return(&roleStore)
+	mockStore.On("System").Return(&systemStore)
+
+	systemStore.On("GetByName", model.MigrationKeyAddManageAgentPermissions).
+		Return(nil, model.NewAppError("test", "missing", nil, "", 404)).Once()
+	systemStore.On("SaveOrUpdate", mock.AnythingOfType("*model.System")).Return(nil).Once()
+
+	// The migration must route through the tolerant save, and the unknown permission
+	// must still be present on the role handed to the store (not stripped).
+	roleStore.On("SavePreservingUnknownPermissions", mock.MatchedBy(func(role *model.Role) bool {
+		return role.Name == model.SystemAdminRoleId && slices.Contains(role.Permissions, unknownPermission)
+	})).Return(func(role *model.Role) *model.Role { return role }, nil).Once()
+
+	appErr := th.App.Srv().doPermissionsMigration(model.MigrationKeyAddManageAgentPermissions, migrationMap, roles)
+	require.Nil(t, appErr, "downgrade migration must not fail fatally on unknown permissions")
+	assert.Contains(t, systemAdminRole.Permissions, unknownPermission, "unknown permission must be preserved across the migration")
+	roleStore.AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 1)
+}
+
 func TestApplyPermissionsMap(t *testing.T) {
 	mainHelper.Parallel(t)
 	tt := []struct {

server/channels/store/localcachelayer/main_test.go

@@ -54,6 +54,7 @@ func getMockStore(t *testing.T) *mocks.Store {
 	fakeRole2 := model.Role{Id: "456", Name: "role-name2"}
 	mockRolesStore := mocks.RoleStore{}
 	mockRolesStore.On("Save", &fakeRole).Return(&model.Role{}, nil)
+	mockRolesStore.On("SavePreservingUnknownPermissions", &fakeRole).Return(&model.Role{}, nil)
 	mockRolesStore.On("Delete", "123").Return(&fakeRole, nil)
 	mockRolesStore.On("GetByName", context.Background(), "role-name").Return(&fakeRole, nil)
 	mockRolesStore.On("GetByNames", []string{"role-name"}).Return([]*model.Role{&fakeRole}, nil)

server/channels/store/localcachelayer/role_layer.go

@@ -44,6 +44,14 @@ func (s LocalCacheRoleStore) Save(role *model.Role) (*model.Role, error) {
 	return s.RoleStore.Save(role)
 }
 
+func (s LocalCacheRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	if role.Name != "" {
+		defer s.rootStore.doInvalidateCacheCluster(s.rootStore.roleCache, role.Name, nil)
+		defer s.rootStore.doClearCacheCluster(s.rootStore.rolePermissionsCache)
+	}
+	return s.RoleStore.SavePreservingUnknownPermissions(role)
+}
+
 func (s LocalCacheRoleStore) GetByName(ctx context.Context, name string) (*model.Role, error) {
 	var role *model.Role
 	if err := s.rootStore.doStandardReadCache(s.rootStore.roleCache, name, &role); err == nil {

server/channels/store/localcachelayer/role_layer_test.go

@@ -46,10 +46,31 @@ func TestRoleStoreCache(t *testing.T) {
 		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger)
 		require.NoError(t, err)
 
-		cachedStore.Role().GetByName(context.Background(), "role-name")
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
 		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
-		cachedStore.Role().Save(&fakeRole)
-		cachedStore.Role().GetByName(context.Background(), "role-name")
+		_, err = cachedStore.Role().Save(&fakeRole)
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "Save", 1)
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
+	})
+
+	t.Run("first call not cached, save preserving unknown permissions, and then not cached again", func(t *testing.T) {
+		mockStore := getMockStore(t)
+		mockCacheProvider := getMockCacheProvider()
+		cachedStore, err := NewLocalCacheLayer(mockStore, nil, nil, mockCacheProvider, logger)
+		require.NoError(t, err)
+
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 1)
+		_, err = cachedStore.Role().SavePreservingUnknownPermissions(&fakeRole)
+		require.NoError(t, err)
+		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "SavePreservingUnknownPermissions", 1)
+		_, err = cachedStore.Role().GetByName(context.Background(), "role-name")
+		require.NoError(t, err)
 		mockStore.Role().(*mocks.RoleStore).AssertNumberOfCalls(t, "GetByName", 2)
 	})
 

server/channels/store/retrylayer/retrylayer.go

@@ -12269,6 +12269,27 @@ func (s *RetryLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 
 }
 
+func (s *RetryLayerRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+
+	tries := 0
+	for {
+		result, err := s.RoleStore.SavePreservingUnknownPermissions(role)
+		if err == nil {
+			return result, nil
+		}
+		if !isRepeatableError(err) {
+			return result, err
+		}
+		tries++
+		if tries >= 3 {
+			err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
+			return result, err
+		}
+		timepkg.Sleep(100 * timepkg.Millisecond)
+	}
+
+}
+
 func (s *RetryLayerScheduledPostStore) CreateScheduledPost(scheduledPost *model.ScheduledPost) (*model.ScheduledPost, error) {
 
 	tries := 0

server/channels/store/sqlstore/role_store.go

@@ -13,6 +13,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/mattermost/mattermost/server/v8/channels/store"
 )
 
@@ -99,10 +100,59 @@ func newSqlRoleStore(sqlStore *SqlStore) store.RoleStore {
 	return &s
 }
 
-func (s *SqlRoleStore) Save(role *model.Role) (_ *model.Role, err error) {
+func (s *SqlRoleStore) Save(role *model.Role) (*model.Role, error) {
+	return s.save(role, false)
+}
+
+// SavePreservingUnknownPermissions behaves like Save but tolerates and persists
+// permissions this server build does not recognize. See the RoleStore interface
+// and MM-68830 for the downgrade scenario this protects against.
+func (s *SqlRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	return s.save(role, true)
+}
+
+// validateForSave validates the role before it is persisted. When
+// preserveUnknownPermissions is true, permissions unknown to this server build are
+// logged and excluded from the validation (but left untouched on the role so they
+// are still persisted), rather than causing the save to fail.
+func (s *SqlRoleStore) validateForSave(role *model.Role, preserveUnknownPermissions bool) error {
+	roleToValidate := role
+
+	if preserveUnknownPermissions {
+		if unknown := role.UnknownPermissions(); len(unknown) > 0 {
+			s.Logger().Warn(
+				"Preserving role permissions not recognized by this server version (server likely downgraded from a newer release)",
+				mlog.String("role", role.Name),
+				mlog.Array("permissions", unknown),
+			)
+
+			unknownSet := make(map[string]bool, len(unknown))
+			for _, permission := range unknown {
+				unknownSet[permission] = true
+			}
+			known := make([]string, 0, len(role.Permissions))
+			for _, permission := range role.Permissions {
+				if !unknownSet[permission] {
+					known = append(known, permission)
+				}
+			}
+
+			roleCopy := role.Clone()
+			roleCopy.Permissions = known
+			roleToValidate = roleCopy
+		}
+	}
+
+	if err := roleToValidate.IsValidWithoutId(); err != nil {
+		return store.NewErrInvalidInput("Role", "<any>", err.Error())
+	}
+	return nil
+}
+
+func (s *SqlRoleStore) save(role *model.Role, preserveUnknownPermissions bool) (*model.Role, error) {
 	// Check the role is valid before proceeding.
-	if err = role.IsValidWithoutId(); err != nil {
-		return nil, store.NewErrInvalidInput("Role", "<any>", err.Error())
+	if err := s.validateForSave(role, preserveUnknownPermissions); err != nil {
+		return nil, err
 	}
 
 	if role.Id == "" {

server/channels/store/sqlstore/role_store_test.go

@@ -6,9 +6,43 @@ package sqlstore
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/request"
+	"github.com/mattermost/mattermost/server/v8/channels/store"
 	"github.com/mattermost/mattermost/server/v8/channels/store/storetest"
 )
 
 func TestRoleStore(t *testing.T) {
 	StoreTestWithSqlStore(t, storetest.TestRoleStore)
 }
+
+// TestSqlRoleStoreCreateRoleValidates guards a regression: createRole must validate
+// the role itself. It is called directly by scheme_store (bypassing Save/save and
+// their validation), so removing its own validation would silently let invalid roles
+// through that path (see MM-68830 review).
+func TestSqlRoleStoreCreateRoleValidates(t *testing.T) {
+	StoreTestWithSqlStore(t, func(t *testing.T, rctx request.CTX, ss store.Store, s storetest.SqlStore) {
+		roleStore := ss.Role().(*SqlRoleStore)
+
+		transaction, err := roleStore.GetMaster().Begin()
+		require.NoError(t, err)
+		defer func() { _ = transaction.Rollback() }()
+
+		// A role carrying a permission this build does not recognize must be rejected
+		// by createRole, just as it is by Save. createRole does not tolerate unknown
+		// permissions; only the migration's SavePreservingUnknownPermissions path does.
+		invalid := &model.Role{
+			Name:        model.NewId(),
+			DisplayName: model.NewId(),
+			Description: model.NewId(),
+			Permissions: []string{"manage_own_agent_from_the_future"},
+		}
+
+		_, err = roleStore.createRole(invalid, transaction)
+		require.Error(t, err, "createRole must reject unknown permissions")
+		var invErr *store.ErrInvalidInput
+		require.ErrorAs(t, err, &invErr)
+	})
+}

server/channels/store/store.go

@@ -863,6 +863,10 @@ type PluginStore interface {
 
 type RoleStore interface {
 	Save(role *model.Role) (*model.Role, error)
+	// SavePreservingUnknownPermissions behaves like Save but tolerates and preserves
+	// permissions not recognized by this server build instead of rejecting the role.
+	// Unrecognized permissions are logged (see MM-68830).
+	SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error)
 	Get(roleID string) (*model.Role, error)
 	GetAll() ([]*model.Role, error)
 	GetByName(ctx context.Context, name string) (*model.Role, error)

server/channels/store/storetest/mocks/RoleStore.go

@@ -304,6 +304,36 @@ func (_m *RoleStore) Save(role *model.Role) (*model.Role, error) {
 	return r0, r1
 }
 
+// SavePreservingUnknownPermissions provides a mock function with given fields: role
+func (_m *RoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	ret := _m.Called(role)
+
+	if len(ret) == 0 {
+		panic("no return value specified for SavePreservingUnknownPermissions")
+	}
+
+	var r0 *model.Role
+	var r1 error
+	if rf, ok := ret.Get(0).(func(*model.Role) (*model.Role, error)); ok {
+		return rf(role)
+	}
+	if rf, ok := ret.Get(0).(func(*model.Role) *model.Role); ok {
+		r0 = rf(role)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.Role)
+		}
+	}
+
+	if rf, ok := ret.Get(1).(func(*model.Role) error); ok {
+		r1 = rf(role)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // NewRoleStore creates a new instance of RoleStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
 // The first argument is typically a *testing.T value.
 func NewRoleStore(t interface {

server/channels/store/storetest/role_store.go

@@ -18,6 +18,7 @@ import (
 
 func TestRoleStore(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {
 	t.Run("Save", func(t *testing.T) { testRoleStoreSave(t, rctx, ss) })
+	t.Run("SavePreservingUnknownPermissions", func(t *testing.T) { testRoleStoreSavePreservingUnknownPermissions(t, rctx, ss) })
 	t.Run("Get", func(t *testing.T) { testRoleStoreGet(t, rctx, ss) })
 	t.Run("GetAll", func(t *testing.T) { testRoleStoreGetAll(t, rctx, ss) })
 	t.Run("GetByName", func(t *testing.T) { testRoleStoreGetByName(t, rctx, ss) })
@@ -103,6 +104,73 @@ func testRoleStoreSave(t *testing.T, rctx request.CTX, ss store.Store) {
 	assert.Error(t, err)
 }
 
+func testRoleStoreSavePreservingUnknownPermissions(t *testing.T, _ request.CTX, ss store.Store) {
+	// A role whose permissions are all valid for this build saves and round-trips
+	// unchanged, just like Save.
+	t.Run("preserves known permissions like Save", func(t *testing.T) {
+		r := &model.Role{
+			Name:          model.NewId(),
+			DisplayName:   model.NewId(),
+			Description:   model.NewId(),
+			Permissions:   []string{"invite_user", "add_user_to_team"},
+			SchemeManaged: false,
+		}
+
+		saved, err := ss.Role().SavePreservingUnknownPermissions(r)
+		require.NoError(t, err)
+		assert.Equal(t, r.Permissions, saved.Permissions)
+	})
+
+	// The downgrade scenario from MM-68830: an existing role gains a permission this
+	// build does not recognize (written by a newer release before the downgrade). The
+	// migration re-saves every role; Save would reject the unknown permission, but
+	// SavePreservingUnknownPermissions must keep it so it is not lost on a future upgrade.
+	t.Run("tolerates and persists unknown permissions on update", func(t *testing.T) {
+		unknown := "manage_own_agent_from_the_future"
+
+		// Create the role as a known-good role first (the pre-downgrade state).
+		existing, err := ss.Role().Save(&model.Role{
+			Name:        model.NewId(),
+			DisplayName: model.NewId(),
+			Description: model.NewId(),
+			Permissions: []string{"invite_user"},
+		})
+		require.NoError(t, err)
+
+		// Simulate the newer release having added an unknown permission to the role.
+		existing.Permissions = append(existing.Permissions, unknown)
+
+		// Sanity check: the regular Save rejects the unknown permission.
+		_, err = ss.Role().Save(existing)
+		require.Error(t, err)
+
+		saved, err := ss.Role().SavePreservingUnknownPermissions(existing)
+		require.NoError(t, err)
+		assert.Contains(t, saved.Permissions, unknown, "unknown permission should be preserved")
+
+		// It must actually be persisted, not just returned.
+		fetched, err := ss.Role().Get(saved.Id)
+		require.NoError(t, err)
+		assert.Contains(t, fetched.Permissions, unknown)
+	})
+
+	// Tolerating unknown permissions must not mask genuine structural problems.
+	t.Run("still rejects structurally invalid roles", func(t *testing.T) {
+		r := &model.Role{
+			Name:          "invalid-name",
+			DisplayName:   model.NewId(),
+			Description:   model.NewId(),
+			Permissions:   []string{"manage_own_agent_from_the_future"},
+			SchemeManaged: false,
+		}
+
+		_, err := ss.Role().SavePreservingUnknownPermissions(r)
+		require.Error(t, err)
+		var invErr *store.ErrInvalidInput
+		require.ErrorAs(t, err, &invErr)
+	})
+}
+
 func testRoleStoreGetAll(t *testing.T, rctx request.CTX, ss store.Store) {
 	prev, err := ss.Role().GetAll()
 	require.NoError(t, err)

server/channels/store/timerlayer/timerlayer.go

@@ -9727,6 +9727,22 @@ func (s *TimerLayerRoleStore) Save(role *model.Role) (*model.Role, error) {
 	return result, err
 }
 
+func (s *TimerLayerRoleStore) SavePreservingUnknownPermissions(role *model.Role) (*model.Role, error) {
+	start := time.Now()
+
+	result, err := s.RoleStore.SavePreservingUnknownPermissions(role)
+
+	elapsed := float64(time.Since(start)) / float64(time.Second)
+	if s.Root.Metrics != nil {
+		success := "false"
+		if err == nil {
+			success = "true"
+		}
+		s.Root.Metrics.ObserveStoreMethodDuration("RoleStore.SavePreservingUnknownPermissions", success, elapsed)
+	}
+	return result, err
+}
+
 func (s *TimerLayerScheduledPostStore) CreateScheduledPost(scheduledPost *model.ScheduledPost) (*model.ScheduledPost, error) {
 	start := time.Now()
 

server/public/model/role.go

@@ -428,6 +428,19 @@ type Role struct {
 	SchemeId      *string  `json:"scheme_id"`
 }
 
+func (r *Role) Clone() *Role {
+	rCopy := *r
+	if r.Permissions != nil {
+		rCopy.Permissions = make([]string, len(r.Permissions))
+		copy(rCopy.Permissions, r.Permissions)
+	}
+	if r.SchemeId != nil {
+		schemeId := *r.SchemeId
+		rCopy.SchemeId = &schemeId
+	}
+	return &rCopy
+}
+
 func (r *Role) Auditable() map[string]any {
 	return map[string]any{
 		"id":             r.Id,
@@ -802,6 +815,16 @@ func (r *Role) IsValidWithoutId() error {
 		return fmt.Errorf("role description exceeds maximum length of %d", RoleDescriptionMaxLength)
 	}
 
+	if unknown := r.UnknownPermissions(); len(unknown) > 0 {
+		return fmt.Errorf("unknown permissions: %s", strings.Join(unknown, ", "))
+	}
+
+	return nil
+}
+
+// UnknownPermissions returns the permissions on the role that are not present in
+// AllPermissions or DeprecatedPermissions (see MM-68830).
+func (r *Role) UnknownPermissions() []string {
 	check := func(perms []*Permission, permission string) bool {
 		for _, p := range perms {
 			if permission == p.Id {
@@ -810,13 +833,14 @@ func (r *Role) IsValidWithoutId() error {
 		}
 		return false
 	}
+
+	var unknown []string
 	for _, permission := range r.Permissions {
 		if !check(AllPermissions, permission) && !check(DeprecatedPermissions, permission) {
-			return fmt.Errorf("unknown permission %q", permission)
+			unknown = append(unknown, permission)
 		}
 	}
-
-	return nil
+	return unknown
 }
 
 func CleanRoleNames(roleNames []string) ([]string, bool) {

server/public/model/role_test.go

@@ -430,6 +430,79 @@ func TestRoleIsValidWithoutId(t *testing.T) {
 	})
 }
 
+func TestRoleUnknownPermissions(t *testing.T) {
+	t.Run("returns nil when all permissions are known", func(t *testing.T) {
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, PermissionCreateEmojis.Id},
+		}
+		assert.Empty(t, r.UnknownPermissions())
+	})
+
+	t.Run("tolerates deprecated permissions", func(t *testing.T) {
+		require.NotEmpty(t, DeprecatedPermissions)
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, DeprecatedPermissions[0].Id},
+		}
+		assert.Empty(t, r.UnknownPermissions())
+	})
+
+	t.Run("returns only the permissions this build does not recognize", func(t *testing.T) {
+		r := &Role{
+			Permissions: []string{PermissionCreatePost.Id, "manage_own_agent_from_the_future", "another_unknown"},
+		}
+		assert.ElementsMatch(t, []string{"manage_own_agent_from_the_future", "another_unknown"}, r.UnknownPermissions())
+	})
+
+	t.Run("empty permissions yields no unknowns", func(t *testing.T) {
+		assert.Empty(t, (&Role{}).UnknownPermissions())
+	})
+}
+
+func TestRoleClone(t *testing.T) {
+	schemeId := NewId()
+	original := &Role{
+		Id:            NewId(),
+		Name:          "test_role",
+		DisplayName:   "Test Role",
+		Description:   "desc",
+		CreateAt:      1000,
+		UpdateAt:      2000,
+		DeleteAt:      0,
+		Permissions:   []string{"invite_user", "add_user_to_team"},
+		SchemeManaged: true,
+		BuiltIn:       false,
+		SchemeId:      &schemeId,
+	}
+
+	t.Run("clone equals original", func(t *testing.T) {
+		cloned := original.Clone()
+		assert.Equal(t, original, cloned)
+	})
+
+	t.Run("permissions are deep copied", func(t *testing.T) {
+		cloned := original.Clone()
+		cloned.Permissions[0] = "mutated"
+		assert.Equal(t, "invite_user", original.Permissions[0])
+	})
+
+	t.Run("scheme id pointer is deep copied", func(t *testing.T) {
+		cloned := original.Clone()
+		require.NotSame(t, original.SchemeId, cloned.SchemeId)
+		*cloned.SchemeId = NewId()
+		assert.Equal(t, schemeId, *original.SchemeId)
+	})
+
+	t.Run("nil permissions stays nil", func(t *testing.T) {
+		r := &Role{}
+		assert.Nil(t, r.Clone().Permissions)
+	})
+
+	t.Run("nil scheme id stays nil", func(t *testing.T) {
+		r := &Role{}
+		assert.Nil(t, r.Clone().SchemeId)
+	})
+}
+
 func TestRoleIsValid(t *testing.T) {
 	validRole := func() *Role {
 		return &Role{