fix(test): stop the RBAC integration suite from sharing the seeded admin - #144
Merged
Merged
Conversation
require-permission.itest.ts reassigned the shared admin account to Viewer, then restored Administrator at the end. That only holds if suites run one at a time. Vitest runs test files in parallel against the same database, so any suite reading the admin's role inside that window sees the read-only Viewer set instead. This is what broke Integration (DB) on the deps branch: roles/route.itest.ts reported the admin missing exactly the ten non-read permissions of the SOC Analyst preset, which is the Viewer set. Forcing the admin to Viewer and running that suite reproduces the CI output character for character. The suite now seeds its own company and asserts on that company's Viewer role. It never touched the admin user for its assertions anyway, the reassignment was dead weight that only created the race.
WhiteMuush
force-pushed
the
fix/itest-shared-admin-race
branch
from
August 11, 2026 12:59
fcbece6 to
0710499
Compare
Owner
Author
|
Retargeted onto Reason: the two pull requests each tripped a gate the other one fixes. #142 was red on Stacking them breaks the cycle: this merges into #142, then #142 merges into |
WhiteMuush
added a commit
that referenced
this pull request
Aug 11, 2026
…142) * chore(deps): port the august bumps to develop and fix the audit gate Dependabot opened #139, #140 and #141 against main, which is 64 commits behind develop. Their lockfiles conflict with develop (next-auth removed, better-auth added), so the bumps are redone here against the develop tree. Ported: - github/codeql-action 4.37.4 to 4.37.6 - @aws-sdk/client-identitystore, @base-ui/react, lucide-react, next 16.3.0 - @types/node, @types/pg, eslint-config-next, tsx Left out: @tanstack/react-table 8 to 9. It is a breaking major that renames the row model factories (getCoreRowModel to createCoreRowModel) and needs a migration of EmployeeTable.tsx. Tracked separately. The Dependency audit job was already failing on develop before these PRs, on two high advisories reaching us through transitives: - js-yaml 4.3.0 via @eslint/eslintrc (GHSA-5p4m-2wfm-xmqj) - nanoid 3.3.16 via postcss (GHSA-2v37-7h3g-55p8) Both are pinned through overrides, so npm audit --audit-level=high is clean. * chore(next): opt out of the AGENTS.md rewrite added in next 16.3 Next 16.3 ships an `agentRules` option, on by default, that makes `next dev` append a Next-authored block to AGENTS.md on every run. Two problems here: the block contains a non-ASCII character, which the pre-push ASCII gate rejects, and the working tree goes dirty on each dev start. AGENTS.md is ours, so the option is turned off. * fix(test): stop the RBAC integration suite from sharing the seeded admin (#144) require-permission.itest.ts reassigned the shared admin account to Viewer, then restored Administrator at the end. That only holds if suites run one at a time. Vitest runs test files in parallel against the same database, so any suite reading the admin's role inside that window sees the read-only Viewer set instead. This is what broke Integration (DB) on the deps branch: roles/route.itest.ts reported the admin missing exactly the ten non-read permissions of the SOC Analyst preset, which is the Viewer set. Forcing the admin to Viewer and running that suite reproduces the CI output character for character. The suite now seeds its own company and asserts on that company's Viewer role. It never touched the admin user for its assertions anyway, the reassignment was dead weight that only created the race.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the flaky
Integration (DB)job. Surfaced on #142, but the race predates it and has nothing to do with dependencies.The race
src/lib/rbac/require-permission.itest.tsreassigned the shared seeded admin (admin@datashield.local) to the Viewer preset, then restored Administrator at the end of the test. Its own comment said "Restore Administrator so other suites keep working", which only holds if suites run one at a time.Vitest runs test files in parallel against the same database. Any suite reading
admin.roleIdinside that window sees the read-only Viewer set.How we know that is what happened
On #142,
src/app/api/roles/route.itest.tsfailed with the admin missing these ten permissions:alerts:assign,alerts:status,alerts:comment,alerts:close,alerts:remediate,employees:scan,register:manage,register:evidence,reports:export,dashboard:customizeThat is exactly the SOC Analyst preset minus its
:readentries, in other words precisely what is left over when the actor holds onlyREAD_ONLY, the Viewer definition.Forcing the admin to Viewer and running that one suite reproduces the CI assertion character for character, same ten strings in the same order. The narrow window does not reproduce by chance on a fast machine: 8 consecutive local runs of the unmodified suite came out green.
The fix
The suite now creates its own company, seeds the presets there, and asserts on that company's Viewer role. It never used the admin user for its assertions in the first place, so the reassignment was dead weight that only created the race.
Verified locally
npm run test:integrationagainst a postgres:16 container: 11 files, 19 tests, pass, three consecutive runsnpx tsc --noEmit: passnpm run lint: passMerge order
Merge this first, then rebase #142 so its
Integration (DB)job goes green.