fix: ISSUE-162 by GitAddRemote · Pull Request #165 · GitAddRemote/station

GitAddRemote · 2026-05-16T17:49:43Z

Summary

Replaces all invented camelCase permission keys in roles.seed.ts (e.g. canViewOrganization, canInviteUsers) with correct OrgPermission enum values (can_view_org_inventory, etc.) — the keys the backend actually enforces
Makes seedRoles() upsert permissions on existing roles rather than skipping them, so re-running pnpm seed heals a broken database without data loss
Preserves user-customized role descriptions; only known legacy seeded descriptions are replaced
Guards demo data seeding (test org, demo user, role assignment) behind NODE_ENV !== 'production' so pnpm seed is safe to run against a production database
Fixes ISSUE-162

Root Cause

Two disconnected permission systems were never reconciled. roles.seed.ts used camelCase keys invented outside the OrgPermission enum; the backend only checks snake_case OrgPermission keys. Every user assigned Owner, Admin, Director, Inventory Manager, Member, or Viewer had zero valid permissions, causing 403s on all inventory endpoints.

Permission Matrix (post-fix)

Role	can_view	can_edit	can_admin	can_view_shared
Owner	✓	✓	✓	✓
Admin	✓	✓	✓	✓
Director	✓	✓	✓	✓
Inventory Manager	✓	✓	✓	✓
Member	✓	✗	✗	✓
Viewer	✓	✗	✗	✗

Test plan

Run pnpm seed against an existing database — all six roles should log ✓ Updated role: <name>
Verify database: SELECT name, permissions FROM role ORDER BY name — all roles show OrgPermission snake_case keys
Log in as the demo user (Owner role) and switch to org view — inventory loads without 403
Re-run pnpm seed a second time — idempotent, no changes logged, no errors
Run NODE_ENV=production pnpm seed (dry-run against dev DB) — confirm demo org/user/role-assignment steps are skipped

…n 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

Copilot

Pull request overview

This PR fixes seeded role permissions by switching default role permission keys to the backend-enforced OrgPermission enum values and making existing seeded roles update their permissions when the seeder is re-run.

Changes:

Replaces legacy camelCase seeded role permissions with OrgPermission enum-backed snake_case keys.
Updates existing seeded roles’ permissions instead of skipping them.
Keeps seed behavior focused on Owner/Admin/Member/Viewer roles.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`backend/src/database/seeds/roles.seed.ts`	Updates default seeded role permission objects to use `OrgPermission` enum keys.
`backend/src/database/seeds/database-seeder.service.ts`	Changes role seeding to save updated permissions for roles that already exist.

Comments suppressed due to low confidence (2)

backend/src/database/seeds/database-seeder.service.ts:98

This new save path is not reflected in the existing DatabaseSeederService spec: the “handle existing data gracefully” test currently asserts that rolesRepository.save is not called when roles already exist, so CI will fail unless that test is updated to cover the permission-update behavior.

        await this.rolesRepository.save(existingRole);
        this.logger.info(`  ✓ Updated permissions for role: ${roleData.name}`);

backend/src/database/seeds/database-seeder.service.ts:98

Updating role permissions here does not invalidate the cached per-user permission sets. PermissionsService caches permissions:user:* entries for 15 minutes, so users who already have a cached empty permission set can continue receiving 403s after pnpm seed until the cache expires or is flushed.

        await this.rolesRepository.save(existingRole);
        this.logger.info(`  ✓ Updated permissions for role: ${roleData.name}`);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…n 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

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

- 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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

backend/src/database/seeds/database-seeder.service.ts:153

Using Redis KEYS for permission invalidation scans the entire Redis keyspace and can block the server while the seed runs. Since this Redis database is shared with other cache entries, use an incremental scan or maintain a permission-cache index/namespace instead of KEYS.

      const keys = await client.keys(PERMISSION_CACHE_PATTERN);

…ation (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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

backend/src/database/seeds/database-seeder.service.ts:173

Using Redis KEYS here performs a blocking scan of the entire Redis database, even with this prefix pattern. Running the seeder against a production Redis instance with many auth/game/org cache entries can stall other application traffic; use a cursor-based SCAN/batched delete approach for permission-key invalidation instead.

        const keys = await client.keys(PERMISSION_CACHE_PATTERN);
        if (keys.length > 0) {
          await client.del(...keys);

backend/src/database/seeds/database-seeder.service.ts:173

This assumes the Redis client's del API is variadic. The configured cache-manager-redis-yet package uses node-redis, where multi-key deletion is passed as an array, so spreading keys can leave all but the first permission cache entry undeleted and users may keep stale permissions after seeding. Delete in batches using the client API's expected key-array form (or normalize per client type).

        const keys = await client.keys(PERMISSION_CACHE_PATTERN);
        if (keys.length > 0) {
          await client.del(...keys);

…t (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

…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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

…ment cache pattern (ISSUE-162) - Type ROLE_DESCRIPTIONS as Record<DefaultRoleName, string> 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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

… (ISSUE-162) - Drop the explicit Record<string, ...> 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

- 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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

…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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

backend/src/database/seeds/roles.seed.ts:13

This description implies the role can edit/administer member-shared items, but the seeded permissions only grant CAN_VIEW_MEMBER_SHARED_ITEMS; the backend route for shared items only checks that view permission. If exposed through the roles API/UI, it overstates what this role can do for member-shared items.

    'Full inventory access. Can view, edit, and administer organization inventory and member shared items.',

backend/src/database/seeds/roles.seed.ts:15

This description implies the role can edit/administer member-shared items, but the seeded permissions only grant CAN_VIEW_MEMBER_SHARED_ITEMS; the backend route for shared items only checks that view permission. If exposed through the roles API/UI, it overstates what this role can do for member-shared items.

    'Full inventory access. Can view, edit, and administer organization inventory and member shared items.',

backend/src/database/seeds/roles.seed.ts:17

This description implies the role can edit/administer member-shared items, but the seeded permissions only grant CAN_VIEW_MEMBER_SHARED_ITEMS; the backend route for shared items only checks that view permission. If exposed through the roles API/UI, it overstates what this role can do for member-shared items.

    'Full inventory access. Can view, edit, and administer organization inventory and member shared items.',

…ion scope (ISSUE-162) - 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)

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

…nvalidation (ISSUE-162) 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.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

…r stale-perms+custom-desc (ISSUE-162) - 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)

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings May 16, 2026 17:49

GitAddRemote added bug Something isn't working backend Backend services and logic database Schema, migrations, indexing security Security, auth, and permissions labels May 16, 2026

Copilot started reviewing on behalf of GitAddRemote May 16, 2026 17:50 View session

Copilot AI reviewed May 16, 2026

View reviewed changes

Comment thread backend/src/database/seeds/database-seeder.service.ts Outdated

Comment thread backend/src/database/seeds/roles.seed.ts Outdated

GitAddRemote requested a review from Copilot May 16, 2026 19:56

Copilot started reviewing on behalf of GitAddRemote May 16, 2026 19:56 View session

Copilot AI reviewed May 16, 2026

View reviewed changes

GitAddRemote requested a review from Copilot May 17, 2026 01:23

Copilot started reviewing on behalf of GitAddRemote May 17, 2026 01:24 View session