From 952c8f21594365e86ecb57844172ba1fb284827a Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 12:33:11 -0400 Subject: [PATCH 01/14] fix: replace dead camelCase permission keys with OrgPermission enum in role seed (ISSUE-162) - Replace all camelCase permission keys in roles.seed.ts with correct OrgPermission enum values (can_view_org_inventory, etc.) - Import OrgPermission from permissions.constants.ts as single source of truth - Make seedRoles() upsert permissions on existing roles so re-running the seeder heals a broken database rather than silently skipping - Owner and Admin get full inventory access; Member gets view+shared; Viewer gets view-only --- .../database/seeds/database-seeder.service.ts | 4 +- backend/src/database/seeds/roles.seed.ts | 72 +++++-------------- 2 files changed, 20 insertions(+), 56 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 1446a99..2ab7ae6 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -93,7 +93,9 @@ export class DatabaseSeederService { await this.rolesRepository.save(role); this.logger.info(` ✓ Created role: ${role.name}`); } else { - this.logger.info(` ⊙ Role already exists: ${roleData.name}`); + existingRole.permissions = roleData.permissions ?? {}; + await this.rolesRepository.save(existingRole); + this.logger.info(` ✓ Updated permissions for role: ${roleData.name}`); } } } diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index cee4f53..5e50092 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -1,4 +1,5 @@ import { Role } from '../../modules/roles/role.entity'; +import { OrgPermission } from '../../modules/permissions/permissions.constants'; export const defaultRoles: Partial[] = [ { @@ -6,79 +7,40 @@ export const defaultRoles: Partial[] = [ description: 'Full access to organization. Can delete organization and manage all settings.', permissions: { - // Organization management - canDeleteOrganization: true, - canEditOrganization: true, - canViewOrganization: true, - - // User management - canInviteUsers: true, - canRemoveUsers: true, - canEditUserRoles: true, - canViewUsers: true, - - // Role management - canCreateRoles: true, - canEditRoles: true, - canDeleteRoles: true, - canViewRoles: true, - - // Settings - canManageSettings: true, - canViewSettings: true, + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, }, { name: 'Admin', description: 'Administrative access. Can manage users and settings.', permissions: { - // Organization management - canEditOrganization: true, - canViewOrganization: true, - - // User management - canInviteUsers: true, - canRemoveUsers: true, - canEditUserRoles: true, - canViewUsers: true, - - // Role management - canViewRoles: true, - - // Settings - canManageSettings: true, - canViewSettings: true, + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, }, { name: 'Member', description: 'Standard member access. Can view and participate.', permissions: { - // Organization management - canViewOrganization: true, - - // User management - canViewUsers: true, - - // Role management - canViewRoles: true, - - // Settings - canViewSettings: true, + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, }, { name: 'Viewer', description: 'Read-only access. Can only view information.', permissions: { - // Organization management - canViewOrganization: true, - - // User management - canViewUsers: true, - - // Settings - canViewSettings: true, + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: false, }, }, ]; From e2a1b4516f089eab85d2568f998e2fac93f953b9 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 15:36:40 -0400 Subject: [PATCH 02/14] fix: merge role permissions on upsert, flush cache, and add permission key tests (ISSUE-162) - Merge seed permissions into existing role JSONB instead of replacing, preserving any custom permissions added via the roles API - Inject CacheManager and call clear() after role permission updates so stale cached permission sets are immediately invalidated - Fix spec: update "existing data" test to expect rolesRepository.save is called and cacheManager.clear is invoked on upsert - Add test asserting all seeded role permission keys are valid OrgPermission enum values to prevent silent regression to dead camelCase keys --- .../seeds/database-seeder.service.spec.ts | 79 ++++++++++++++++++- .../database/seeds/database-seeder.service.ts | 19 ++++- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index d56285e..8763a03 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -1,6 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; import { getLoggerToken } from 'nestjs-pino'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Repository } from 'typeorm'; import { DatabaseSeederService } from './database-seeder.service'; import { Role } from '../../modules/roles/role.entity'; @@ -8,6 +9,8 @@ import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; import { UserOrganizationRole } from '../../modules/user-organization-roles/user-organization-role.entity'; import { Game } from '../../modules/games/game.entity'; +import { OrgPermission } from '../../modules/permissions/permissions.constants'; +import { defaultRoles } from './roles.seed'; // Mock bcrypt module jest.mock('bcrypt', () => ({ @@ -27,6 +30,9 @@ describe('DatabaseSeederService', () => { error: jest.fn(), debug: jest.fn(), }; + const mockCacheManager = { + clear: jest.fn().mockResolvedValue(undefined), + }; let loggerErrorSpy: jest.Mock; const mockGame = { @@ -72,6 +78,7 @@ describe('DatabaseSeederService', () => { mockLogger.warn.mockClear(); mockLogger.error.mockClear(); mockLogger.debug.mockClear(); + mockCacheManager.clear.mockClear(); loggerErrorSpy = mockLogger.error; const module: TestingModule = await Test.createTestingModule({ @@ -81,6 +88,10 @@ describe('DatabaseSeederService', () => { provide: getLoggerToken(DatabaseSeederService.name), useValue: mockLogger, }, + { + provide: CACHE_MANAGER, + useValue: mockCacheManager, + }, { provide: getRepositoryToken(Role), useValue: { @@ -192,13 +203,16 @@ describe('DatabaseSeederService', () => { await expect(service.seedAll()).resolves.toBeUndefined(); }); - it('should handle existing data gracefully', async () => { + it('should update role permissions and clear cache when roles already exist', async () => { jest .spyOn(gamesRepository, 'findOne') .mockResolvedValue(mockGame as unknown as Game); jest .spyOn(rolesRepository, 'findOne') .mockResolvedValue(mockRole as unknown as Role); + jest + .spyOn(rolesRepository, 'save') + .mockResolvedValue(mockRole as unknown as Role); jest .spyOn(organizationsRepository, 'findOne') .mockResolvedValue(mockOrganization as unknown as Organization); @@ -212,10 +226,71 @@ describe('DatabaseSeederService', () => { await expect(service.seedAll()).resolves.toBeUndefined(); expect(gamesRepository.save).not.toHaveBeenCalled(); - expect(rolesRepository.save).not.toHaveBeenCalled(); + expect(rolesRepository.save).toHaveBeenCalled(); expect(organizationsRepository.save).not.toHaveBeenCalled(); expect(usersRepository.save).not.toHaveBeenCalled(); expect(userOrgRolesRepository.save).not.toHaveBeenCalled(); + expect(mockCacheManager.clear).toHaveBeenCalled(); + }); + + it('should seed roles with correct OrgPermission keys', async () => { + const savedRoles: Partial[] = []; + + jest.spyOn(rolesRepository, 'findOne').mockResolvedValue(null); + jest + .spyOn(rolesRepository, 'create') + .mockImplementation((data) => data as Role); + jest.spyOn(rolesRepository, 'save').mockImplementation(async (role) => { + savedRoles.push(role as Partial); + return role as Role; + }); + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + jest + .spyOn(gamesRepository, 'create') + .mockReturnValue(mockGame as unknown as Game); + jest + .spyOn(gamesRepository, 'save') + .mockResolvedValue(mockGame as unknown as Game); + jest.spyOn(organizationsRepository, 'findOne').mockResolvedValue(null); + jest + .spyOn(organizationsRepository, 'create') + .mockReturnValue(mockOrganization as unknown as Organization); + jest + .spyOn(organizationsRepository, 'save') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest.spyOn(usersRepository, 'findOne').mockResolvedValue(null); + jest + .spyOn(usersRepository, 'create') + .mockReturnValue(mockUser as unknown as User); + jest + .spyOn(usersRepository, 'save') + .mockResolvedValue(mockUser as unknown as User); + jest.spyOn(userOrgRolesRepository, 'findOne').mockResolvedValue(null); + jest + .spyOn(userOrgRolesRepository, 'create') + .mockReturnValue(mockUserOrgRole as unknown as UserOrganizationRole); + jest + .spyOn(userOrgRolesRepository, 'save') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + const validPermissionKeys = new Set(Object.values(OrgPermission)); + const seededRoleNames = savedRoles.map((r) => r.name); + + expect(seededRoleNames).toEqual( + expect.arrayContaining(defaultRoles.map((r) => r.name)), + ); + + for (const role of savedRoles) { + const keys = Object.keys(role.permissions ?? {}); + for (const key of keys) { + expect(validPermissionKeys).toContain(key); + } + } }); it('should throw error on failure', async () => { diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 2ab7ae6..ef1b256 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -1,7 +1,9 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Inject } from '@nestjs/common'; import { InjectPinoLogger, PinoLogger } from 'nestjs-pino'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { Cache } from 'cache-manager'; import { Role } from '../../modules/roles/role.entity'; import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; @@ -15,6 +17,8 @@ export class DatabaseSeederService { constructor( @InjectPinoLogger(DatabaseSeederService.name) private readonly logger: PinoLogger, + @Inject(CACHE_MANAGER) + private readonly cacheManager: Cache, @InjectRepository(Role) private rolesRepository: Repository, @InjectRepository(Organization) @@ -83,6 +87,8 @@ export class DatabaseSeederService { private async seedRoles(): Promise { this.logger.info('Seeding roles...'); + let permissionsUpdated = false; + for (const roleData of defaultRoles) { const existingRole = await this.rolesRepository.findOne({ where: { name: roleData.name }, @@ -93,11 +99,20 @@ export class DatabaseSeederService { await this.rolesRepository.save(role); this.logger.info(` ✓ Created role: ${role.name}`); } else { - existingRole.permissions = roleData.permissions ?? {}; + existingRole.permissions = { + ...existingRole.permissions, + ...roleData.permissions, + }; await this.rolesRepository.save(existingRole); this.logger.info(` ✓ Updated permissions for role: ${roleData.name}`); + permissionsUpdated = true; } } + + if (permissionsUpdated) { + await this.cacheManager.clear(); + this.logger.info(' ✓ Cleared permission cache'); + } } private async seedTestOrganization(): Promise { From 05ae539cc5ad0e4d247ba9070a94d09e65c60bf6 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 20:31:59 -0400 Subject: [PATCH 03/14] fix: address PR 165 review feedback (ISSUE-162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Strip legacy camelCase permission keys on merge to heal stale DB rows - Add idempotency check — skip save and cache flush when permissions unchanged - Replace cacheManager.clear() with targeted Redis key scan on permissions:user:* pattern, with fallback to clear() for in-memory store - Derive roles.seed.ts from DEFAULT_ROLE_PERMISSIONS to eliminate duplicated permission matrix - Add Owner and Viewer entries to DEFAULT_ROLE_PERMISSIONS - Assert saved permissions shape and per-role values in existing-role test - Add idempotency test asserting no save/cache-clear when permissions are current - Add privilege-escalation assertions for Member and Viewer roles --- .../seeds/database-seeder.service.spec.ts | 105 ++++++++++++++++-- .../database/seeds/database-seeder.service.ts | 60 ++++++++-- backend/src/database/seeds/roles.seed.ts | 69 +++++------- .../permissions/permissions.constants.ts | 22 +++- 4 files changed, 192 insertions(+), 64 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 8763a03..7501d83 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -9,7 +9,10 @@ import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; import { UserOrganizationRole } from '../../modules/user-organization-roles/user-organization-role.entity'; import { Game } from '../../modules/games/game.entity'; -import { OrgPermission } from '../../modules/permissions/permissions.constants'; +import { + OrgPermission, + DEFAULT_ROLE_PERMISSIONS, +} from '../../modules/permissions/permissions.constants'; import { defaultRoles } from './roles.seed'; // Mock bcrypt module @@ -204,15 +207,28 @@ describe('DatabaseSeederService', () => { }); it('should update role permissions and clear cache when roles already exist', async () => { + const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; + jest .spyOn(gamesRepository, 'findOne') .mockResolvedValue(mockGame as unknown as Game); - jest - .spyOn(rolesRepository, 'findOne') - .mockResolvedValue(mockRole as unknown as Role); - jest + + // Return a role with legacy camelCase keys for every findOne call so all + // existing-role paths exercise the strip-and-merge logic. + jest.spyOn(rolesRepository, 'findOne').mockImplementation( + async () => + ({ + ...mockRole, + permissions: { + canViewOrganization: true, + canInviteUsers: true, + }, + }) as unknown as Role, + ); + + const saveSpy = jest .spyOn(rolesRepository, 'save') - .mockResolvedValue(mockRole as unknown as Role); + .mockImplementation(async (role) => role as Role); jest .spyOn(organizationsRepository, 'findOne') .mockResolvedValue(mockOrganization as unknown as Organization); @@ -231,9 +247,54 @@ describe('DatabaseSeederService', () => { expect(usersRepository.save).not.toHaveBeenCalled(); expect(userOrgRolesRepository.save).not.toHaveBeenCalled(); expect(mockCacheManager.clear).toHaveBeenCalled(); + + // The first saved role is Owner — assert legacy keys were stripped and seed + // permissions were applied correctly. + const savedOwner = saveSpy.mock.calls[0][0] as Partial; + expect(savedOwner.permissions).not.toHaveProperty('canViewOrganization'); + expect(savedOwner.permissions).not.toHaveProperty('canInviteUsers'); + expect(savedOwner.permissions).toEqual( + expect.objectContaining(ownerSeedData.permissions), + ); }); - it('should seed roles with correct OrgPermission keys', async () => { + it('should not save or clear cache when permissions are already up to date', async () => { + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + + // Return the exact seed permissions for each role so no change is detected. + jest + .spyOn(rolesRepository, 'findOne') + .mockImplementation(async (opts) => { + const name = (opts as { where: { name: string } }).where.name; + const seedRole = defaultRoles.find((r) => r.name === name); + return { + ...mockRole, + name, + permissions: { ...(seedRole?.permissions ?? {}) }, + } as unknown as Role; + }); + jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await expect(service.seedAll()).resolves.toBeUndefined(); + + expect(rolesRepository.save).not.toHaveBeenCalled(); + expect(mockCacheManager.clear).not.toHaveBeenCalled(); + }); + + it('should seed roles with correct OrgPermission keys and per-role permission values', async () => { const savedRoles: Partial[] = []; jest.spyOn(rolesRepository, 'findOne').mockResolvedValue(null); @@ -285,12 +346,42 @@ describe('DatabaseSeederService', () => { expect.arrayContaining(defaultRoles.map((r) => r.name)), ); + // All permission keys must be valid OrgPermission enum values. for (const role of savedRoles) { const keys = Object.keys(role.permissions ?? {}); for (const key of keys) { expect(validPermissionKeys).toContain(key); } } + + // Per-role permission values must match DEFAULT_ROLE_PERMISSIONS exactly + // to prevent privilege escalation (e.g. Viewer/Member gaining edit/admin). + for (const role of savedRoles) { + const expected = DEFAULT_ROLE_PERMISSIONS[role.name!]; + if (expected) { + expect(role.permissions).toEqual(expected); + } + } + + // Restricted roles must not have elevated permissions. + const memberRole = savedRoles.find((r) => r.name === 'Member'); + const viewerRole = savedRoles.find((r) => r.name === 'Viewer'); + + expect( + memberRole?.permissions?.[OrgPermission.CAN_EDIT_ORG_INVENTORY], + ).toBe(false); + expect( + memberRole?.permissions?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY], + ).toBe(false); + expect( + viewerRole?.permissions?.[OrgPermission.CAN_EDIT_ORG_INVENTORY], + ).toBe(false); + expect( + viewerRole?.permissions?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY], + ).toBe(false); + expect( + viewerRole?.permissions?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + ).toBe(false); }); it('should throw error on failure', async () => { diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index ef1b256..2807822 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -9,9 +9,13 @@ import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; import { UserOrganizationRole } from '../../modules/user-organization-roles/user-organization-role.entity'; import { Game } from '../../modules/games/game.entity'; +import { OrgPermission } from '../../modules/permissions/permissions.constants'; import { defaultRoles } from './roles.seed'; import * as bcrypt from 'bcrypt'; +const VALID_PERMISSION_KEYS = new Set(Object.values(OrgPermission)); +const PERMISSION_CACHE_PATTERN = 'permissions:user:*'; + @Injectable() export class DatabaseSeederService { constructor( @@ -99,22 +103,62 @@ export class DatabaseSeederService { await this.rolesRepository.save(role); this.logger.info(` ✓ Created role: ${role.name}`); } else { - existingRole.permissions = { - ...existingRole.permissions, - ...roleData.permissions, - }; - await this.rolesRepository.save(existingRole); - this.logger.info(` ✓ Updated permissions for role: ${roleData.name}`); - permissionsUpdated = true; + // Strip legacy/invalid keys from existing permissions, then merge seed permissions. + const sanitizedExisting = Object.fromEntries( + Object.entries(existingRole.permissions ?? {}).filter(([k]) => + VALID_PERMISSION_KEYS.has(k), + ), + ); + const merged = { ...sanitizedExisting, ...roleData.permissions }; + + // Only save if permissions actually changed (idempotency). + if ( + JSON.stringify(existingRole.permissions) !== JSON.stringify(merged) + ) { + existingRole.permissions = merged; + await this.rolesRepository.save(existingRole); + this.logger.info( + ` ✓ Updated permissions for role: ${roleData.name}`, + ); + permissionsUpdated = true; + } else { + this.logger.info( + ` ⊙ Permissions unchanged for role: ${roleData.name}`, + ); + } } } if (permissionsUpdated) { - await this.cacheManager.clear(); + await this.invalidatePermissionCache(); this.logger.info(' ✓ Cleared permission cache'); } } + private async invalidatePermissionCache(): Promise { + // Attempt targeted invalidation of only permission keys to avoid evicting + // unrelated cached data (games, orgs, etc.) from the shared Redis database. + const store = ( + this.cacheManager as unknown as { + store?: { + client?: { + keys?: (pattern: string) => Promise; + del?: (...keys: string[]) => Promise; + }; + }; + } + ).store; + const client = store?.client; + if (client?.keys && client?.del) { + const keys = await client.keys(PERMISSION_CACHE_PATTERN); + if (keys.length > 0) { + await client.del(...keys); + } + } else { + await this.cacheManager.clear(); + } + } + private async seedTestOrganization(): Promise { this.logger.info('Seeding test organization...'); diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index 5e50092..0c6ffa3 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -1,46 +1,27 @@ import { Role } from '../../modules/roles/role.entity'; -import { OrgPermission } from '../../modules/permissions/permissions.constants'; +import { DEFAULT_ROLE_PERMISSIONS } from '../../modules/permissions/permissions.constants'; -export const defaultRoles: Partial[] = [ - { - name: 'Owner', - description: - 'Full access to organization. Can delete organization and manage all settings.', - permissions: { - [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, - [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, - [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, - [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, - }, - }, - { - name: 'Admin', - description: 'Administrative access. Can manage users and settings.', - permissions: { - [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, - [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, - [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, - [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, - }, - }, - { - name: 'Member', - description: 'Standard member access. Can view and participate.', - permissions: { - [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, - [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, - [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, - [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, - }, - }, - { - name: 'Viewer', - description: 'Read-only access. Can only view information.', - permissions: { - [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, - [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, - [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, - [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: false, - }, - }, -]; +const ROLE_DESCRIPTIONS: Record = { + Owner: + 'Full access to organization. Can delete organization and manage all settings.', + Admin: 'Administrative access. Can manage users and settings.', + Member: 'Standard member access. Can view and participate.', + Viewer: 'Read-only access. Can only view information.', +}; + +export const defaultRoles: Partial[] = Object.entries( + DEFAULT_ROLE_PERMISSIONS, +) + .filter( + ( + entry, + ): entry is [ + keyof typeof ROLE_DESCRIPTIONS, + (typeof DEFAULT_ROLE_PERMISSIONS)[string], + ] => entry[0] in ROLE_DESCRIPTIONS, + ) + .map(([name, permissions]) => ({ + name, + description: ROLE_DESCRIPTIONS[name], + permissions, + })); diff --git a/backend/src/modules/permissions/permissions.constants.ts b/backend/src/modules/permissions/permissions.constants.ts index 522bb60..42080fc 100644 --- a/backend/src/modules/permissions/permissions.constants.ts +++ b/backend/src/modules/permissions/permissions.constants.ts @@ -25,13 +25,13 @@ export const DEFAULT_ROLE_PERMISSIONS: Record< string, Record > = { - Member: { + Owner: { [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, - [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, - [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, - 'Inventory Manager': { + Admin: { [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, @@ -43,12 +43,24 @@ export const DEFAULT_ROLE_PERMISSIONS: Record< [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, - Admin: { + 'Inventory Manager': { [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: true, [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, }, + Member: { + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: true, + }, + Viewer: { + [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, + [OrgPermission.CAN_EDIT_ORG_INVENTORY]: false, + [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, + [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: false, + }, }; /** From 6c05a23e954b78feb022e57bffab6a33697900ce Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 22:07:34 -0400 Subject: [PATCH 04/14] fix: preserve custom permission keys and fix cache-manager v7 invalidation (ISSUE-162) - Replace VALID_PERMISSION_KEYS filter with LEGACY_PERMISSION_KEYS denylist so only known dead camelCase keys are stripped on merge; unknown custom keys survive - Rewrite invalidatePermissionCache to use cacheManager.stores (Keyv[]) per the cache-manager v7 API instead of the non-existent .store.client path; falls back to cacheManager.clear() only when no Redis-backed store is found - Add test asserting custom (non-legacy) keys are preserved after merge - Add stores: [] to mockCacheManager so unit tests exercise the correct fallback path --- .../seeds/database-seeder.service.spec.ts | 51 ++++++++++++++ .../database/seeds/database-seeder.service.ts | 68 ++++++++++++------- 2 files changed, 96 insertions(+), 23 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 7501d83..e6fbdec 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -35,6 +35,9 @@ describe('DatabaseSeederService', () => { }; const mockCacheManager = { clear: jest.fn().mockResolvedValue(undefined), + // cache-manager v7: stores is an array of Keyv instances. No Redis client + // in unit tests, so the code falls back to cacheManager.clear(). + stores: [], }; let loggerErrorSpy: jest.Mock; @@ -294,6 +297,54 @@ describe('DatabaseSeederService', () => { expect(mockCacheManager.clear).not.toHaveBeenCalled(); }); + it('should preserve custom (non-legacy) permission keys when merging', async () => { + const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; + const customKey = OrgPermission.CAN_VIEW_ORG_INVENTORY; // valid non-legacy key + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + + jest.spyOn(rolesRepository, 'findOne').mockImplementation( + async () => + ({ + ...mockRole, + permissions: { + // Legacy key — should be stripped. + canViewOrganization: true, + // Custom valid key — should be preserved. + [customKey]: false, + }, + }) as unknown as Role, + ); + + const saveSpy = jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + const savedOwner = saveSpy.mock.calls[0][0] as Partial; + // Legacy key must be gone. + expect(savedOwner.permissions).not.toHaveProperty('canViewOrganization'); + // Custom key must survive (seed value wins if it also defines this key, + // so just assert the key is present). + expect(savedOwner.permissions).toHaveProperty(customKey); + // Seed permissions are applied on top. + expect(savedOwner.permissions).toEqual( + expect.objectContaining(ownerSeedData.permissions), + ); + }); + it('should seed roles with correct OrgPermission keys and per-role permission values', async () => { const savedRoles: Partial[] = []; diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 2807822..5b0c19c 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -9,11 +9,26 @@ import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; import { UserOrganizationRole } from '../../modules/user-organization-roles/user-organization-role.entity'; import { Game } from '../../modules/games/game.entity'; -import { OrgPermission } from '../../modules/permissions/permissions.constants'; import { defaultRoles } from './roles.seed'; import * as bcrypt from 'bcrypt'; -const VALID_PERMISSION_KEYS = new Set(Object.values(OrgPermission)); +// Known legacy camelCase keys introduced before the OrgPermission enum. Only +// these are stripped on merge — unknown custom keys are preserved. +const LEGACY_PERMISSION_KEYS = new Set([ + 'canDeleteOrganization', + 'canEditOrganization', + 'canViewOrganization', + 'canInviteUsers', + 'canRemoveUsers', + 'canEditUserRoles', + 'canViewUsers', + 'canCreateRoles', + 'canEditRoles', + 'canDeleteRoles', + 'canViewRoles', + 'canManageSettings', + 'canViewSettings', +]); const PERMISSION_CACHE_PATTERN = 'permissions:user:*'; @Injectable() @@ -103,10 +118,11 @@ export class DatabaseSeederService { await this.rolesRepository.save(role); this.logger.info(` ✓ Created role: ${role.name}`); } else { - // Strip legacy/invalid keys from existing permissions, then merge seed permissions. + // Strip only known legacy camelCase keys from existing permissions; + // unknown custom keys are preserved and carried forward. const sanitizedExisting = Object.fromEntries( - Object.entries(existingRole.permissions ?? {}).filter(([k]) => - VALID_PERMISSION_KEYS.has(k), + Object.entries(existingRole.permissions ?? {}).filter( + ([k]) => !LEGACY_PERMISSION_KEYS.has(k), ), ); const merged = { ...sanitizedExisting, ...roleData.permissions }; @@ -136,25 +152,31 @@ export class DatabaseSeederService { } private async invalidatePermissionCache(): Promise { - // Attempt targeted invalidation of only permission keys to avoid evicting - // unrelated cached data (games, orgs, etc.) from the shared Redis database. - const store = ( - this.cacheManager as unknown as { - store?: { - client?: { - keys?: (pattern: string) => Promise; - del?: (...keys: string[]) => Promise; - }; - }; - } - ).store; - const client = store?.client; - if (client?.keys && client?.del) { - const keys = await client.keys(PERMISSION_CACHE_PATTERN); - if (keys.length > 0) { - await client.del(...keys); + // cache-manager v7 exposes backing stores via `cacheManager.stores` (Keyv[]). + // Each Keyv wraps a store adapter; for cache-manager-redis-yet the adapter + // exposes `.client` (the ioredis/node-redis client) which supports KEYS/DEL. + type RedisClient = { + keys?: (pattern: string) => Promise; + del?: (...keys: string[]) => Promise; + }; + type KeyvLike = { store?: { client?: RedisClient } }; + + const stores: KeyvLike[] = + (this.cacheManager as unknown as { stores?: KeyvLike[] }).stores ?? []; + + let invalidated = false; + for (const keyv of stores) { + const client = keyv.store?.client; + if (client?.keys && client?.del) { + const keys = await client.keys(PERMISSION_CACHE_PATTERN); + if (keys.length > 0) { + await client.del(...keys); + } + invalidated = true; } - } else { + } + + if (!invalidated) { await this.cacheManager.clear(); } } From e65221313e1b49551032f730b8558a46907eb2dc Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 22:32:07 -0400 Subject: [PATCH 05/14] fix: use SCAN/DEL array for Redis invalidation and fix custom-key test (ISSUE-162) - Switch from KEYS+variadic DEL to non-blocking scanIterator+DEL(array) to match node-redis 4.x API and avoid full-keyspace scans on large Redis instances - Fix custom-key preservation test to use an unknown key not in the seed matrix so a regression that drops non-OrgPermission keys actually fails the test - Add test covering the Redis store branch (SCAN/DEL path) and asserting that cacheManager.clear() is not called when a Redis store handles invalidation --- .../seeds/database-seeder.service.spec.ts | 66 +++++++++++++++++-- .../database/seeds/database-seeder.service.ts | 23 +++++-- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index e6fbdec..15d67e2 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -299,7 +299,9 @@ describe('DatabaseSeederService', () => { it('should preserve custom (non-legacy) permission keys when merging', async () => { const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; - const customKey = OrgPermission.CAN_VIEW_ORG_INVENTORY; // valid non-legacy key + // Use a key that is not in the seed matrix for Owner so it cannot be + // overwritten by the merge and truly exercises the custom-key path. + const customKey = 'custom:guild:officer' as unknown as OrgPermission; jest .spyOn(gamesRepository, 'findOne') @@ -312,8 +314,8 @@ describe('DatabaseSeederService', () => { permissions: { // Legacy key — should be stripped. canViewOrganization: true, - // Custom valid key — should be preserved. - [customKey]: false, + // Unknown custom key — should be preserved. + [customKey]: true, }, }) as unknown as Role, ); @@ -336,15 +338,67 @@ describe('DatabaseSeederService', () => { const savedOwner = saveSpy.mock.calls[0][0] as Partial; // Legacy key must be gone. expect(savedOwner.permissions).not.toHaveProperty('canViewOrganization'); - // Custom key must survive (seed value wins if it also defines this key, - // so just assert the key is present). - expect(savedOwner.permissions).toHaveProperty(customKey); + // Unknown custom key must survive. + expect(savedOwner.permissions).toHaveProperty(customKey, true); // Seed permissions are applied on top. expect(savedOwner.permissions).toEqual( expect.objectContaining(ownerSeedData.permissions), ); }); + it('should use targeted Redis SCAN/DEL when a Redis store is present', async () => { + const matchedKeys = [ + 'permissions:user:1:org:1', + 'permissions:user:2:org:1', + ]; + const mockRedisClient = { + scanIterator: jest + .fn() + .mockImplementation(() => matchedKeys[Symbol.iterator]()), + del: jest.fn().mockResolvedValue(undefined), + }; + const mockRedisStore = { store: { client: mockRedisClient } }; + + // Temporarily add a Redis-like store to the cache manager. + (mockCacheManager as { stores: unknown[] }).stores = [mockRedisStore]; + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + jest.spyOn(rolesRepository, 'findOne').mockImplementation( + async () => + ({ + ...mockRole, + permissions: { canViewOrganization: true }, + }) as unknown as Role, + ); + jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + expect(mockRedisClient.scanIterator).toHaveBeenCalledWith({ + MATCH: 'permissions:user:*', + COUNT: 100, + }); + expect(mockRedisClient.del).toHaveBeenCalledWith(matchedKeys); + // Global clear must NOT be called when Redis store handles it. + expect(mockCacheManager.clear).not.toHaveBeenCalled(); + + // Restore empty stores for subsequent tests. + (mockCacheManager as { stores: unknown[] }).stores = []; + }); + it('should seed roles with correct OrgPermission keys and per-role permission values', async () => { const savedRoles: Partial[] = []; diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 5b0c19c..c61789d 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -154,10 +154,15 @@ export class DatabaseSeederService { private async invalidatePermissionCache(): Promise { // cache-manager v7 exposes backing stores via `cacheManager.stores` (Keyv[]). // Each Keyv wraps a store adapter; for cache-manager-redis-yet the adapter - // exposes `.client` (the ioredis/node-redis client) which supports KEYS/DEL. + // exposes `.client` (the node-redis 4.x client). + // Use SCAN (non-blocking cursor iteration) instead of KEYS to avoid stalling + // Redis on large keyspaces. node-redis 4.x del() takes an array, not variadic args. type RedisClient = { - keys?: (pattern: string) => Promise; - del?: (...keys: string[]) => Promise; + scanIterator?: (opts: { + MATCH: string; + COUNT: number; + }) => AsyncIterable; + del?: (keys: string[]) => Promise; }; type KeyvLike = { store?: { client?: RedisClient } }; @@ -167,10 +172,16 @@ export class DatabaseSeederService { let invalidated = false; for (const keyv of stores) { const client = keyv.store?.client; - if (client?.keys && client?.del) { - const keys = await client.keys(PERMISSION_CACHE_PATTERN); + if (client?.scanIterator && client?.del) { + const keys: string[] = []; + for await (const key of client.scanIterator({ + MATCH: PERMISSION_CACHE_PATTERN, + COUNT: 100, + })) { + keys.push(key); + } if (keys.length > 0) { - await client.del(...keys); + await client.del(keys); } invalidated = true; } From 24fd0dd4c660b4d13c15c8d9df7123c79fe9d0ab Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 22:36:37 -0400 Subject: [PATCH 06/14] fix: order-insensitive idempotency check and independent role matrix assertions (ISSUE-162) - Replace JSON.stringify idempotency comparison with key-sorted stringify so JSONB key-order variance after a round trip cannot cause spurious saves/cache clears - Replace circular DEFAULT_ROLE_PERMISSIONS comparison in tests with hardcoded per-role assertions so bugs in the seed matrix itself are actually caught - Remove unused DEFAULT_ROLE_PERMISSIONS import from spec --- .../seeds/database-seeder.service.spec.ts | 80 +++++++++++++------ .../database/seeds/database-seeder.service.ts | 8 +- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 15d67e2..e38c76d 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -9,10 +9,7 @@ import { Organization } from '../../modules/organizations/organization.entity'; import { User } from '../../modules/users/user.entity'; import { UserOrganizationRole } from '../../modules/user-organization-roles/user-organization-role.entity'; import { Game } from '../../modules/games/game.entity'; -import { - OrgPermission, - DEFAULT_ROLE_PERMISSIONS, -} from '../../modules/permissions/permissions.constants'; +import { OrgPermission } from '../../modules/permissions/permissions.constants'; import { defaultRoles } from './roles.seed'; // Mock bcrypt module @@ -459,33 +456,64 @@ describe('DatabaseSeederService', () => { } } - // Per-role permission values must match DEFAULT_ROLE_PERMISSIONS exactly - // to prevent privilege escalation (e.g. Viewer/Member gaining edit/admin). - for (const role of savedRoles) { - const expected = DEFAULT_ROLE_PERMISSIONS[role.name!]; - if (expected) { - expect(role.permissions).toEqual(expected); - } - } - - // Restricted roles must not have elevated permissions. - const memberRole = savedRoles.find((r) => r.name === 'Member'); - const viewerRole = savedRoles.find((r) => r.name === 'Viewer'); + // Independent per-role assertions — hardcoded so a bug in the seed matrix + // itself (e.g. Owner losing view access, Viewer gaining edit) is caught. + const byName = Object.fromEntries( + savedRoles.map((r) => [r.name, r.permissions]), + ); + // Owner and Admin: full access + expect(byName['Owner']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Owner']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Owner']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( + true, + ); expect( - memberRole?.permissions?.[OrgPermission.CAN_EDIT_ORG_INVENTORY], - ).toBe(false); - expect( - memberRole?.permissions?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY], - ).toBe(false); + byName['Owner']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + ).toBe(true); + expect(byName['Admin']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Admin']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Admin']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( + true, + ); expect( - viewerRole?.permissions?.[OrgPermission.CAN_EDIT_ORG_INVENTORY], - ).toBe(false); + byName['Admin']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + ).toBe(true); + + // Member: can view and view shared, but not edit or admin + expect(byName['Member']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Member']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( + false, + ); + expect(byName['Member']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( + false, + ); expect( - viewerRole?.permissions?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY], - ).toBe(false); + byName['Member']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + ).toBe(true); + + // Viewer: can only view inventory, nothing else + expect(byName['Viewer']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( + true, + ); + expect(byName['Viewer']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( + false, + ); + expect(byName['Viewer']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( + false, + ); expect( - viewerRole?.permissions?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + byName['Viewer']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], ).toBe(false); }); diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index c61789d..31efebc 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -128,9 +128,11 @@ export class DatabaseSeederService { const merged = { ...sanitizedExisting, ...roleData.permissions }; // Only save if permissions actually changed (idempotency). - if ( - JSON.stringify(existingRole.permissions) !== JSON.stringify(merged) - ) { + // Use key-sorted stringify because JSONB does not preserve key order, + // so a naive stringify can differ even for semantically equal objects. + const sortedKeys = (obj: Record) => + JSON.stringify(obj, Object.keys(obj).sort()); + if (sortedKeys(existingRole.permissions ?? {}) !== sortedKeys(merged)) { existingRole.permissions = merged; await this.rolesRepository.save(existingRole); this.logger.info( From e333e54982508b5d74c0acb0073092e96fc2e16d Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 22:47:08 -0400 Subject: [PATCH 07/14] fix: batch Redis DEL, add missing roles, align role descriptions (ISSUE-162) - Delete matched permission cache keys in batches of 100 as the SCAN iterator yields to keep memory and command size bounded - Add Director and Inventory Manager to the seed ROLE_DESCRIPTIONS so all six roles defined in DEFAULT_ROLE_PERMISSIONS are created on seed/reseed - Align role descriptions with the actual inventory-only OrgPermission matrix - Update Redis batch test to assert two DEL calls (100 + 50) over 150 keys - Expand role matrix test to cover Director and Inventory Manager --- .../seeds/database-seeder.service.spec.ts | 70 ++++++++++--------- .../database/seeds/database-seeder.service.ts | 15 ++-- backend/src/database/seeds/roles.seed.ts | 13 ++-- 3 files changed, 55 insertions(+), 43 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index e38c76d..3a768de 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -343,11 +343,12 @@ describe('DatabaseSeederService', () => { ); }); - it('should use targeted Redis SCAN/DEL when a Redis store is present', async () => { - const matchedKeys = [ - 'permissions:user:1:org:1', - 'permissions:user:2:org:1', - ]; + it('should use targeted Redis SCAN/DEL in batches when a Redis store is present', async () => { + // Simulate 150 matched keys so two DEL batches are issued (100 + 50). + const matchedKeys = Array.from( + { length: 150 }, + (_, i) => `permissions:user:${i}:org:1`, + ); const mockRedisClient = { scanIterator: jest .fn() @@ -356,7 +357,6 @@ describe('DatabaseSeederService', () => { }; const mockRedisStore = { store: { client: mockRedisClient } }; - // Temporarily add a Redis-like store to the cache manager. (mockCacheManager as { stores: unknown[] }).stores = [mockRedisStore]; jest @@ -388,11 +388,18 @@ describe('DatabaseSeederService', () => { MATCH: 'permissions:user:*', COUNT: 100, }); - expect(mockRedisClient.del).toHaveBeenCalledWith(matchedKeys); - // Global clear must NOT be called when Redis store handles it. + // Two batches: first 100 keys, then the remaining 50. + expect(mockRedisClient.del).toHaveBeenCalledTimes(2); + expect(mockRedisClient.del).toHaveBeenNthCalledWith( + 1, + matchedKeys.slice(0, 100), + ); + expect(mockRedisClient.del).toHaveBeenNthCalledWith( + 2, + matchedKeys.slice(100), + ); expect(mockCacheManager.clear).not.toHaveBeenCalled(); - // Restore empty stores for subsequent tests. (mockCacheManager as { stores: unknown[] }).stores = []; }); @@ -462,31 +469,26 @@ describe('DatabaseSeederService', () => { savedRoles.map((r) => [r.name, r.permissions]), ); - // Owner and Admin: full access - expect(byName['Owner']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( - true, - ); - expect(byName['Owner']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( - true, - ); - expect(byName['Owner']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( - true, - ); - expect( - byName['Owner']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], - ).toBe(true); - expect(byName['Admin']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( - true, - ); - expect(byName['Admin']?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( - true, - ); - expect(byName['Admin']?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( - true, - ); - expect( - byName['Admin']?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], - ).toBe(true); + // Full-access roles: Owner, Admin, Director, Inventory Manager + for (const roleName of [ + 'Owner', + 'Admin', + 'Director', + 'Inventory Manager', + ]) { + expect(byName[roleName]?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( + true, + ); + expect(byName[roleName]?.[OrgPermission.CAN_EDIT_ORG_INVENTORY]).toBe( + true, + ); + expect(byName[roleName]?.[OrgPermission.CAN_ADMIN_ORG_INVENTORY]).toBe( + true, + ); + expect( + byName[roleName]?.[OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS], + ).toBe(true); + } // Member: can view and view shared, but not edit or admin expect(byName['Member']?.[OrgPermission.CAN_VIEW_ORG_INVENTORY]).toBe( diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 31efebc..5d33f88 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -171,19 +171,24 @@ export class DatabaseSeederService { const stores: KeyvLike[] = (this.cacheManager as unknown as { stores?: KeyvLike[] }).stores ?? []; + const BATCH_SIZE = 100; let invalidated = false; for (const keyv of stores) { const client = keyv.store?.client; if (client?.scanIterator && client?.del) { - const keys: string[] = []; + let batch: string[] = []; for await (const key of client.scanIterator({ MATCH: PERMISSION_CACHE_PATTERN, - COUNT: 100, + COUNT: BATCH_SIZE, })) { - keys.push(key); + batch.push(key); + if (batch.length >= BATCH_SIZE) { + await client.del(batch); + batch = []; + } } - if (keys.length > 0) { - await client.del(keys); + if (batch.length > 0) { + await client.del(batch); } invalidated = true; } diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index 0c6ffa3..3b2e926 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -3,10 +3,15 @@ import { DEFAULT_ROLE_PERMISSIONS } from '../../modules/permissions/permissions. const ROLE_DESCRIPTIONS: Record = { Owner: - 'Full access to organization. Can delete organization and manage all settings.', - Admin: 'Administrative access. Can manage users and settings.', - Member: 'Standard member access. Can view and participate.', - Viewer: 'Read-only access. Can only view information.', + 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + Admin: + 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + Director: + 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + 'Inventory Manager': + 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + Member: 'Standard member access. Can view inventory and member shared items.', + Viewer: 'Read-only access. Can only view organization inventory.', }; export const defaultRoles: Partial[] = Object.entries( From 1d7e79e3c427d96ba5745382aba6d61012caa715 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 23:31:37 -0400 Subject: [PATCH 08/14] fix: exhaustive role descriptions, update description on reseed, document cache pattern (ISSUE-162) - Type ROLE_DESCRIPTIONS as Record so TypeScript fails at compile time when a new default role lacks a description - Remove the filter that silently dropped roles without descriptions; all roles in DEFAULT_ROLE_PERMISSIONS now map through exhaustively - Seed update path now also patches role description when it differs from the seed value, so existing installs with stale legacy descriptions are healed - Add comment to PERMISSION_CACHE_PATTERN documenting the no-namespace assumption and the action required if a Keyv namespace is ever configured - Fix idempotency test mock to return matching description so no spurious save fires --- .../seeds/database-seeder.service.spec.ts | 4 ++- .../database/seeds/database-seeder.service.ts | 31 +++++++++++++------ backend/src/database/seeds/roles.seed.ts | 31 +++++++++---------- 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 3a768de..eac3e09 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -263,7 +263,8 @@ describe('DatabaseSeederService', () => { .spyOn(gamesRepository, 'findOne') .mockResolvedValue(mockGame as unknown as Game); - // Return the exact seed permissions for each role so no change is detected. + // Return the exact seed permissions AND description for each role so no + // change is detected and the idempotency path skips save entirely. jest .spyOn(rolesRepository, 'findOne') .mockImplementation(async (opts) => { @@ -272,6 +273,7 @@ describe('DatabaseSeederService', () => { return { ...mockRole, name, + description: seedRole?.description ?? mockRole.description, permissions: { ...(seedRole?.permissions ?? {}) }, } as unknown as Role; }); diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 5d33f88..171da4b 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -29,6 +29,9 @@ const LEGACY_PERMISSION_KEYS = new Set([ 'canManageSettings', 'canViewSettings', ]); +// Bare key pattern — no Keyv namespace prefix is configured in app.module.ts, +// so permission keys are stored as-is in Redis. If a namespace is ever added, +// this pattern must be updated to match (e.g. 'namespace:permissions:user:*'). const PERMISSION_CACHE_PATTERN = 'permissions:user:*'; @Injectable() @@ -127,22 +130,30 @@ export class DatabaseSeederService { ); const merged = { ...sanitizedExisting, ...roleData.permissions }; - // Only save if permissions actually changed (idempotency). // Use key-sorted stringify because JSONB does not preserve key order, // so a naive stringify can differ even for semantically equal objects. const sortedKeys = (obj: Record) => JSON.stringify(obj, Object.keys(obj).sort()); - if (sortedKeys(existingRole.permissions ?? {}) !== sortedKeys(merged)) { - existingRole.permissions = merged; + const permissionsChanged = + sortedKeys(existingRole.permissions ?? {}) !== sortedKeys(merged); + const descriptionChanged = + roleData.description !== undefined && + existingRole.description !== roleData.description; + + if (permissionsChanged || descriptionChanged) { + if (permissionsChanged) { + existingRole.permissions = merged; + } + if (descriptionChanged) { + existingRole.description = roleData.description!; + } await this.rolesRepository.save(existingRole); - this.logger.info( - ` ✓ Updated permissions for role: ${roleData.name}`, - ); - permissionsUpdated = true; + this.logger.info(` ✓ Updated role: ${roleData.name}`); + if (permissionsChanged) { + permissionsUpdated = true; + } } else { - this.logger.info( - ` ⊙ Permissions unchanged for role: ${roleData.name}`, - ); + this.logger.info(` ⊙ Role unchanged: ${roleData.name}`); } } } diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index 3b2e926..ceb450b 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -1,7 +1,10 @@ import { Role } from '../../modules/roles/role.entity'; import { DEFAULT_ROLE_PERMISSIONS } from '../../modules/permissions/permissions.constants'; -const ROLE_DESCRIPTIONS: Record = { +// Typed against DEFAULT_ROLE_PERMISSIONS so TypeScript fails at compile time +// if a new default role is added without a corresponding description. +type DefaultRoleName = keyof typeof DEFAULT_ROLE_PERMISSIONS; +const ROLE_DESCRIPTIONS: Record = { Owner: 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', Admin: @@ -14,19 +17,13 @@ const ROLE_DESCRIPTIONS: Record = { Viewer: 'Read-only access. Can only view organization inventory.', }; -export const defaultRoles: Partial[] = Object.entries( - DEFAULT_ROLE_PERMISSIONS, -) - .filter( - ( - entry, - ): entry is [ - keyof typeof ROLE_DESCRIPTIONS, - (typeof DEFAULT_ROLE_PERMISSIONS)[string], - ] => entry[0] in ROLE_DESCRIPTIONS, - ) - .map(([name, permissions]) => ({ - name, - description: ROLE_DESCRIPTIONS[name], - permissions, - })); +export const defaultRoles: Partial[] = ( + Object.entries(DEFAULT_ROLE_PERMISSIONS) as [ + DefaultRoleName, + (typeof DEFAULT_ROLE_PERMISSIONS)[DefaultRoleName], + ][] +).map(([name, permissions]) => ({ + name, + description: ROLE_DESCRIPTIONS[name], + permissions, +})); From 11de4b7d88444f8bf7c3a72ff9db7c3d2ae69f23 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sat, 16 May 2026 23:50:11 -0400 Subject: [PATCH 09/14] fix: preserve literal role-name keys and test description-only reseed (ISSUE-162) - Drop the explicit Record annotation on DEFAULT_ROLE_PERMISSIONS and replace it with a satisfies clause so keyof resolves to the literal union of role names; ROLE_DESCRIPTIONS now truly fails to compile when a role is added without a description - Update ROLE_DESCRIPTIONS comment to reflect the satisfies-based narrowing - Add test: description-only change saves the role but does not clear the cache --- .../seeds/database-seeder.service.spec.ts | 44 +++++++++++++++++++ backend/src/database/seeds/roles.seed.ts | 6 ++- .../permissions/permissions.constants.ts | 12 ++--- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index eac3e09..0117e03 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -258,6 +258,50 @@ describe('DatabaseSeederService', () => { ); }); + it('should update description without clearing cache when only description differs', async () => { + const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + + // Return roles with correct permissions but a stale description. + jest + .spyOn(rolesRepository, 'findOne') + .mockImplementation(async (opts) => { + const name = (opts as { where: { name: string } }).where.name; + const seedRole = defaultRoles.find((r) => r.name === name); + return { + ...mockRole, + name, + description: 'Stale legacy description', + permissions: { ...(seedRole?.permissions ?? {}) }, + } as unknown as Role; + }); + + const saveSpy = jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + // Save must have been called to patch the description. + expect(saveSpy).toHaveBeenCalled(); + const savedOwner = saveSpy.mock.calls[0][0] as Partial; + expect(savedOwner.description).toBe(ownerSeedData.description); + // Permissions did not change so cache must NOT be cleared. + expect(mockCacheManager.clear).not.toHaveBeenCalled(); + }); + it('should not save or clear cache when permissions are already up to date', async () => { jest .spyOn(gamesRepository, 'findOne') diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index ceb450b..a7affa9 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -1,8 +1,10 @@ import { Role } from '../../modules/roles/role.entity'; import { DEFAULT_ROLE_PERMISSIONS } from '../../modules/permissions/permissions.constants'; -// Typed against DEFAULT_ROLE_PERMISSIONS so TypeScript fails at compile time -// if a new default role is added without a corresponding description. +// `keyof typeof DEFAULT_ROLE_PERMISSIONS` is now a literal union of role names +// (not `string`) because the constant uses `satisfies` without a wide type +// annotation. TypeScript will fail to compile if a new role is added to +// DEFAULT_ROLE_PERMISSIONS without a matching entry here. type DefaultRoleName = keyof typeof DEFAULT_ROLE_PERMISSIONS; const ROLE_DESCRIPTIONS: Record = { Owner: diff --git a/backend/src/modules/permissions/permissions.constants.ts b/backend/src/modules/permissions/permissions.constants.ts index 42080fc..54a23e3 100644 --- a/backend/src/modules/permissions/permissions.constants.ts +++ b/backend/src/modules/permissions/permissions.constants.ts @@ -20,11 +20,13 @@ export enum OrgPermission { /** * Default role permission mappings + * + * Declared without an explicit wide type so `keyof typeof DEFAULT_ROLE_PERMISSIONS` + * resolves to the literal union of role name strings rather than `string`. + * The `satisfies` clause still enforces that every value is a complete + * `Record`, giving both narrowing and type-safety. */ -export const DEFAULT_ROLE_PERMISSIONS: Record< - string, - Record -> = { +export const DEFAULT_ROLE_PERMISSIONS = { Owner: { [OrgPermission.CAN_VIEW_ORG_INVENTORY]: true, [OrgPermission.CAN_EDIT_ORG_INVENTORY]: true, @@ -61,7 +63,7 @@ export const DEFAULT_ROLE_PERMISSIONS: Record< [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: false, [OrgPermission.CAN_VIEW_MEMBER_SHARED_ITEMS]: false, }, -}; +} satisfies Record>; /** * Permission descriptions for documentation and UI From cc7ea9164617d07d55091c2012aa946d7d1cd9d6 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sun, 17 May 2026 00:02:05 -0400 Subject: [PATCH 10/14] fix: correct TTL comment and move stores reset into beforeEach - Update PERMISSION_CACHE_PATTERN comment to reflect the actual 15 min TTL used by PermissionsService (was incorrectly noted as 5 min) - Move mockCacheManager.stores reset from end-of-test cleanup into beforeEach so test isolation holds even when a test throws before reaching the cleanup line --- backend/src/database/seeds/database-seeder.service.spec.ts | 3 +-- backend/src/database/seeds/database-seeder.service.ts | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 0117e03..00f7610 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -82,6 +82,7 @@ describe('DatabaseSeederService', () => { mockLogger.error.mockClear(); mockLogger.debug.mockClear(); mockCacheManager.clear.mockClear(); + (mockCacheManager as { stores: unknown[] }).stores = []; loggerErrorSpy = mockLogger.error; const module: TestingModule = await Test.createTestingModule({ @@ -445,8 +446,6 @@ describe('DatabaseSeederService', () => { matchedKeys.slice(100), ); expect(mockCacheManager.clear).not.toHaveBeenCalled(); - - (mockCacheManager as { stores: unknown[] }).stores = []; }); it('should seed roles with correct OrgPermission keys and per-role permission values', async () => { diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 171da4b..7618dfa 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -30,8 +30,9 @@ const LEGACY_PERMISSION_KEYS = new Set([ 'canViewSettings', ]); // Bare key pattern — no Keyv namespace prefix is configured in app.module.ts, -// so permission keys are stored as-is in Redis. If a namespace is ever added, -// this pattern must be updated to match (e.g. 'namespace:permissions:user:*'). +// so permission keys are stored as-is in Redis. TTL is 15 min (900000ms) per +// PermissionsService. If a namespace is ever added, this pattern must be +// updated to match (e.g. 'namespace:permissions:user:*'). const PERMISSION_CACHE_PATTERN = 'permissions:user:*'; @Injectable() From 53218483b1ae7472096f9efc263ef31bf4adab69 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sun, 17 May 2026 00:22:28 -0400 Subject: [PATCH 11/14] fix: guard demo seeding in production, preserve custom descriptions (ISSUE-162) - Skip seedTestOrganization/seedTestUser/seedUserOrganizationRoles when NODE_ENV=production so pnpm seed is safe to run against a production database - Only replace role descriptions that match known legacy seeded values; any other description is treated as a user customization and left untouched - Add LEGACY_ROLE_DESCRIPTIONS set to track replaceable legacy descriptions - Add tests: custom description preserved, production guard skips demo data --- .../seeds/database-seeder.service.spec.ts | 86 ++++++++++++++++++- .../database/seeds/database-seeder.service.ts | 36 ++++++-- 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 00f7610..6717bf4 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -266,7 +266,8 @@ describe('DatabaseSeederService', () => { .spyOn(gamesRepository, 'findOne') .mockResolvedValue(mockGame as unknown as Game); - // Return roles with correct permissions but a stale description. + // Return roles with correct permissions but a known legacy description + // that the seeder is allowed to replace. jest .spyOn(rolesRepository, 'findOne') .mockImplementation(async (opts) => { @@ -275,7 +276,8 @@ describe('DatabaseSeederService', () => { return { ...mockRole, name, - description: 'Stale legacy description', + description: + 'Full access to organization. Can delete organization and manage all settings.', permissions: { ...(seedRole?.permissions ?? {}) }, } as unknown as Role; }); @@ -564,6 +566,86 @@ describe('DatabaseSeederService', () => { ).toBe(false); }); + it('should not overwrite a user-customized role description', async () => { + const customDescription = 'Custom org-specific description set by admin'; + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + + jest + .spyOn(rolesRepository, 'findOne') + .mockImplementation(async (opts) => { + const name = (opts as { where: { name: string } }).where.name; + const seedRole = defaultRoles.find((r) => r.name === name); + return { + ...mockRole, + name, + // Not a legacy description and not the current seed text — custom. + description: customDescription, + permissions: { ...(seedRole?.permissions ?? {}) }, + } as unknown as Role; + }); + + const saveSpy = jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + // Permissions are up to date so no save should occur. + expect(saveSpy).not.toHaveBeenCalled(); + // The custom description must be untouched. + expect(mockCacheManager.clear).not.toHaveBeenCalled(); + }); + + it('should skip demo data seeding in production', async () => { + const originalEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'production'; + + try { + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + jest + .spyOn(rolesRepository, 'findOne') + .mockImplementation(async (opts) => { + const name = (opts as { where: { name: string } }).where.name; + const seedRole = defaultRoles.find((r) => r.name === name); + return { + ...mockRole, + name, + description: seedRole?.description ?? mockRole.description, + permissions: { ...(seedRole?.permissions ?? {}) }, + } as unknown as Role; + }); + jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + + await service.seedAll(); + + // Demo-data repositories must never be touched in production. + expect(organizationsRepository.findOne).not.toHaveBeenCalled(); + expect(usersRepository.findOne).not.toHaveBeenCalled(); + expect(userOrgRolesRepository.findOne).not.toHaveBeenCalled(); + expect(organizationsRepository.save).not.toHaveBeenCalled(); + expect(usersRepository.save).not.toHaveBeenCalled(); + expect(userOrgRolesRepository.save).not.toHaveBeenCalled(); + } finally { + process.env.NODE_ENV = originalEnv; + } + }); + it('should throw error on failure', async () => { jest .spyOn(gamesRepository, 'findOne') diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index 7618dfa..ca7e02e 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -12,6 +12,16 @@ import { Game } from '../../modules/games/game.entity'; import { defaultRoles } from './roles.seed'; import * as bcrypt from 'bcrypt'; +// Known legacy descriptions seeded before the inventory-focused rewrite. Only +// these values are replaced on reseed — any other description is treated as +// a user customization and left untouched. +const LEGACY_ROLE_DESCRIPTIONS = new Set([ + 'Full access to organization. Can delete organization and manage all settings.', + 'Administrative access. Can manage users and settings.', + 'Standard member access. Can view and participate.', + 'Read-only access. Can only view information.', +]); + // Known legacy camelCase keys introduced before the OrgPermission enum. Only // these are stripped on merge — unknown custom keys are preserved. const LEGACY_PERMISSION_KEYS = new Set([ @@ -58,12 +68,20 @@ export class DatabaseSeederService { this.logger.info('🌱 Starting database seeding...'); try { - // Seed in order of dependencies + // Always safe to run in any environment await this.seedGames(); await this.seedRoles(); - await this.seedTestOrganization(); - await this.seedTestUser(); - await this.seedUserOrganizationRoles(); + + // Demo credentials must never be created in production + if (process.env.NODE_ENV !== 'production') { + await this.seedTestOrganization(); + await this.seedTestUser(); + await this.seedUserOrganizationRoles(); + } else { + this.logger.info( + '⊙ Skipping demo data seeding in production environment', + ); + } this.logger.info('✅ Database seeding completed successfully!'); } catch (error) { @@ -137,9 +155,17 @@ export class DatabaseSeederService { JSON.stringify(obj, Object.keys(obj).sort()); const permissionsChanged = sortedKeys(existingRole.permissions ?? {}) !== sortedKeys(merged); + // Only update the description if it is a known legacy seeded value or + // still matches the current seed text (i.e. was never customized). + // Any other description is treated as a user customization and preserved. + const isReplaceableDescription = + existingRole.description !== undefined && + (LEGACY_ROLE_DESCRIPTIONS.has(existingRole.description) || + existingRole.description === roleData.description); const descriptionChanged = roleData.description !== undefined && - existingRole.description !== roleData.description; + existingRole.description !== roleData.description && + isReplaceableDescription; if (permissionsChanged || descriptionChanged) { if (permissionsChanged) { From 7813a4e403c7c81d92a2d050b2d9ae485aa7eed2 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sun, 17 May 2026 00:36:44 -0400 Subject: [PATCH 12/14] fix: correct role and permission descriptions to match actual permission scope (ISSUE-162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Role descriptions for full-access roles now say "view member shared items" rather than "administer member shared items" — CAN_VIEW_MEMBER_SHARED_ITEMS is a view-only permission - CAN_VIEW_ORG_INVENTORY description no longer claims to cover member-shared items; that is a separate permission (CAN_VIEW_MEMBER_SHARED_ITEMS) --- backend/src/database/seeds/roles.seed.ts | 11 ++++++----- .../src/modules/permissions/permissions.constants.ts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/backend/src/database/seeds/roles.seed.ts b/backend/src/database/seeds/roles.seed.ts index a7affa9..55f1c96 100644 --- a/backend/src/database/seeds/roles.seed.ts +++ b/backend/src/database/seeds/roles.seed.ts @@ -8,14 +8,15 @@ import { DEFAULT_ROLE_PERMISSIONS } from '../../modules/permissions/permissions. type DefaultRoleName = keyof typeof DEFAULT_ROLE_PERMISSIONS; const ROLE_DESCRIPTIONS: Record = { Owner: - 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + 'Full inventory access. Can view, edit, and administer organization inventory, and view member shared items.', Admin: - 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + 'Full inventory access. Can view, edit, and administer organization inventory, and view member shared items.', Director: - 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', + 'Full inventory access. Can view, edit, and administer organization inventory, and view member shared items.', 'Inventory Manager': - 'Full inventory access. Can view, edit, and administer organization inventory and member shared items.', - Member: 'Standard member access. Can view inventory and member shared items.', + 'Full inventory access. Can view, edit, and administer organization inventory, and view member shared items.', + Member: + 'Standard member access. Can view organization inventory and member shared items.', Viewer: 'Read-only access. Can only view organization inventory.', }; diff --git a/backend/src/modules/permissions/permissions.constants.ts b/backend/src/modules/permissions/permissions.constants.ts index 54a23e3..593112f 100644 --- a/backend/src/modules/permissions/permissions.constants.ts +++ b/backend/src/modules/permissions/permissions.constants.ts @@ -70,7 +70,7 @@ export const DEFAULT_ROLE_PERMISSIONS = { */ export const PERMISSION_DESCRIPTIONS: Record = { [OrgPermission.CAN_VIEW_ORG_INVENTORY]: - 'View organization-owned inventory and member-shared items', + 'View organization-owned inventory items', [OrgPermission.CAN_EDIT_ORG_INVENTORY]: 'Create, update, and delete organization inventory items', [OrgPermission.CAN_ADMIN_ORG_INVENTORY]: From 9d47ec3ba77f49b199a61b78cfe31ceea09e7633 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sun, 17 May 2026 00:56:37 -0400 Subject: [PATCH 13/14] fix: warn instead of clear() when no Redis store found during cache invalidation (ISSUE-162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seeder runs as a separate process. Calling cacheManager.clear() on the in-memory fallback only clears the seeder's own throwaway cache instance — the running backend's in-memory cache is unreachable from a different process. Replace the silent clear() with a warning that tells operators to restart the backend or wait for TTL expiry. --- .../seeds/database-seeder.service.spec.ts | 15 ++++++++++----- .../src/database/seeds/database-seeder.service.ts | 9 ++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index 6717bf4..eca3304 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -32,9 +32,9 @@ describe('DatabaseSeederService', () => { }; const mockCacheManager = { clear: jest.fn().mockResolvedValue(undefined), - // cache-manager v7: stores is an array of Keyv instances. No Redis client - // in unit tests, so the code falls back to cacheManager.clear(). - stores: [], + // cache-manager v7: stores is an array of Keyv instances. Tests use an + // empty array (no Redis) so the seeder logs a warning instead of clearing. + stores: [] as unknown[], }; let loggerErrorSpy: jest.Mock; @@ -207,7 +207,7 @@ describe('DatabaseSeederService', () => { await expect(service.seedAll()).resolves.toBeUndefined(); }); - it('should update role permissions and clear cache when roles already exist', async () => { + it('should update role permissions and warn when no Redis store is available', async () => { const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; jest @@ -247,7 +247,12 @@ describe('DatabaseSeederService', () => { expect(organizationsRepository.save).not.toHaveBeenCalled(); expect(usersRepository.save).not.toHaveBeenCalled(); expect(userOrgRolesRepository.save).not.toHaveBeenCalled(); - expect(mockCacheManager.clear).toHaveBeenCalled(); + // No Redis store — must warn, not call clear() (which only clears the + // seeder's own throwaway cache, not the running backend's). + expect(mockCacheManager.clear).not.toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('No Redis store found'), + ); // The first saved role is Owner — assert legacy keys were stripped and seed // permissions were applied correctly. diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index ca7e02e..c04c8d9 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -233,7 +233,14 @@ export class DatabaseSeederService { } if (!invalidated) { - await this.cacheManager.clear(); + // No Redis-backed store was found — the running backend may be using an + // in-memory cache (USE_REDIS_CACHE=false or Redis unavailable). In-memory + // caches are per-process: this seeder process cannot reach the backend's + // cache instance. Stale permission entries will expire naturally at TTL + // (15 min) or be cleared on backend restart. + this.logger.warn( + ' ⚠️ No Redis store found — backend in-memory permission cache cannot be invalidated from this process. Restart the backend or wait for TTL expiry.', + ); } } From d751a86cb93953a3931d62555a005ce891179f59 Mon Sep 17 00:00:00 2001 From: gitaddremote Date: Sun, 17 May 2026 01:24:21 -0400 Subject: [PATCH 14/14] fix: accurate cache-clear log, add migration legacy description, cover stale-perms+custom-desc (ISSUE-162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - invalidatePermissionCache() now returns bool; caller only logs success when Redis invalidation actually occurred — no false success message on the in-memory fallback (warn-only) path - Add migration-seeded Inventory Manager description to LEGACY_ROLE_DESCRIPTIONS so reseed correctly updates it on existing databases - Split custom-description test into two cases: permissions current (no save) and permissions stale (save for perms, description preserved) --- .../seeds/database-seeder.service.spec.ts | 50 +++++++++++++++++-- .../database/seeds/database-seeder.service.ts | 12 +++-- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/backend/src/database/seeds/database-seeder.service.spec.ts b/backend/src/database/seeds/database-seeder.service.spec.ts index eca3304..e31dc2c 100644 --- a/backend/src/database/seeds/database-seeder.service.spec.ts +++ b/backend/src/database/seeds/database-seeder.service.spec.ts @@ -571,7 +571,7 @@ describe('DatabaseSeederService', () => { ).toBe(false); }); - it('should not overwrite a user-customized role description', async () => { + it('should not overwrite a user-customized role description when permissions are current', async () => { const customDescription = 'Custom org-specific description set by admin'; jest @@ -586,7 +586,6 @@ describe('DatabaseSeederService', () => { return { ...mockRole, name, - // Not a legacy description and not the current seed text — custom. description: customDescription, permissions: { ...(seedRole?.permissions ?? {}) }, } as unknown as Role; @@ -607,12 +606,55 @@ describe('DatabaseSeederService', () => { await service.seedAll(); - // Permissions are up to date so no save should occur. expect(saveSpy).not.toHaveBeenCalled(); - // The custom description must be untouched. expect(mockCacheManager.clear).not.toHaveBeenCalled(); }); + it('should update stale permissions but preserve a custom description', async () => { + const customDescription = 'Custom org-specific description set by admin'; + const ownerSeedData = defaultRoles.find((r) => r.name === 'Owner')!; + + jest + .spyOn(gamesRepository, 'findOne') + .mockResolvedValue(mockGame as unknown as Game); + + jest.spyOn(rolesRepository, 'findOne').mockImplementation( + async () => + ({ + ...mockRole, + // Stale permissions — will trigger a save. + permissions: { canViewOrganization: true }, + // Custom description — must survive the save untouched. + description: customDescription, + }) as unknown as Role, + ); + + const saveSpy = jest + .spyOn(rolesRepository, 'save') + .mockImplementation(async (role) => role as Role); + jest + .spyOn(organizationsRepository, 'findOne') + .mockResolvedValue(mockOrganization as unknown as Organization); + jest + .spyOn(usersRepository, 'findOne') + .mockResolvedValue(mockUser as unknown as User); + jest + .spyOn(userOrgRolesRepository, 'findOne') + .mockResolvedValue(mockUserOrgRole as unknown as UserOrganizationRole); + + await service.seedAll(); + + // Role must have been saved (permissions were stale). + expect(saveSpy).toHaveBeenCalled(); + const savedOwner = saveSpy.mock.calls[0][0] as Partial; + // Permissions must be updated to current seed values. + expect(savedOwner.permissions).toEqual( + expect.objectContaining(ownerSeedData.permissions), + ); + // Custom description must be preserved — not overwritten by seed text. + expect(savedOwner.description).toBe(customDescription); + }); + it('should skip demo data seeding in production', async () => { const originalEnv = process.env.NODE_ENV; process.env.NODE_ENV = 'production'; diff --git a/backend/src/database/seeds/database-seeder.service.ts b/backend/src/database/seeds/database-seeder.service.ts index c04c8d9..7acf2f4 100644 --- a/backend/src/database/seeds/database-seeder.service.ts +++ b/backend/src/database/seeds/database-seeder.service.ts @@ -20,6 +20,8 @@ const LEGACY_ROLE_DESCRIPTIONS = new Set([ 'Administrative access. Can manage users and settings.', 'Standard member access. Can view and participate.', 'Read-only access. Can only view information.', + // Seeded by migration 1764961461064-SeedInventoryManagerRole + 'Manages organization inventory with full permissions for viewing, editing, and administering items', ]); // Known legacy camelCase keys introduced before the OrgPermission enum. Only @@ -186,12 +188,14 @@ export class DatabaseSeederService { } if (permissionsUpdated) { - await this.invalidatePermissionCache(); - this.logger.info(' ✓ Cleared permission cache'); + const cacheCleared = await this.invalidatePermissionCache(); + if (cacheCleared) { + this.logger.info(' ✓ Cleared permission cache'); + } } } - private async invalidatePermissionCache(): Promise { + private async invalidatePermissionCache(): Promise { // cache-manager v7 exposes backing stores via `cacheManager.stores` (Keyv[]). // Each Keyv wraps a store adapter; for cache-manager-redis-yet the adapter // exposes `.client` (the node-redis 4.x client). @@ -242,6 +246,8 @@ export class DatabaseSeederService { ' ⚠️ No Redis store found — backend in-memory permission cache cannot be invalidated from this process. Restart the backend or wait for TTL expiry.', ); } + + return invalidated; } private async seedTestOrganization(): Promise {