Summary
POST /v1/groups is authorized for any GROUP_MANAGER, so a group manager can create arbitrary new groups.
Why
apps/api/src/auth/ability.factory.ts grants a group manager:
ability.can('manage', 'Group', { id: { in: groupIds } });
JwtAuthGuard evaluates @RouteAccess against the subject type only (ability.can(action, subject) with a subject name, not a row), and CASL ignores a rule's conditions for a type-level check. So the condition is invisible to the guard:
can('create', 'Group') => true can('create', 'User') => false
can('update', 'Group') => true can('manage', 'all') => false
can('delete', 'Group') => true
(Confirmed by building that exact rule set with @casl/prisma and calling can directly.)
Every sibling route survives this because the service re-checks against real rows — accessibleQuery(ability, ...) in the Prisma where, i.e. Layer 2 in .agents/docs/architecture/auth-and-permissions.md. GroupsService.create is the exception: it takes no ability parameter at all, and a create has no existing row to scope against, so nothing narrows it.
Repro
POST /v1/groups with a group manager's access token → 201.
Also reachable through the UI: /admin/groups/create renders for a group manager (no route-level role check exists — see the companion issue), so the form submits successfully and redirects to the group list.
Impact
Bounded, but real. The created group is instance-wide, is connected to every non-repo instrument, occupies the unique-name space, and appears in every admin's group list.
It is not a data-access escalation: permissions are frozen into the JWT at login, so the creator is not a member of the group they just created and cannot read anything through it.
Suggested fix
Either:
- Narrow
@RouteAccess on GroupsController.create to { action: 'manage', subject: 'all' }, matching who the UI intends to expose it to and the sibling PATCH /v1/setup; or
- Give
GroupsService.create an { ability } parameter and check the payload with forcedAppSubject('Group', ...), the way files.service.ts does.
(1) is smaller and matches existing convention. Worth a unit test in src/auth/__tests__/ability.factory.test.ts asserting the deny case either way.
Notes
Found while writing the Playwright suite (branch e2e-tests). testing/src/specs/authorization.spec.ts now has a server-side authorization block asserting the whole family of privileged requests is refused for non-admin roles; POST /v1/groups is deliberately excluded from that table so the suite does not lock in this behaviour. Add it back when this is fixed.
Summary
POST /v1/groupsis authorized for anyGROUP_MANAGER, so a group manager can create arbitrary new groups.Why
apps/api/src/auth/ability.factory.tsgrants a group manager:JwtAuthGuardevaluates@RouteAccessagainst the subject type only (ability.can(action, subject)with a subject name, not a row), and CASL ignores a rule's conditions for a type-level check. So the condition is invisible to the guard:(Confirmed by building that exact rule set with
@casl/prismaand callingcandirectly.)Every sibling route survives this because the service re-checks against real rows —
accessibleQuery(ability, ...)in the Prismawhere, i.e. Layer 2 in.agents/docs/architecture/auth-and-permissions.md.GroupsService.createis the exception: it takes noabilityparameter at all, and a create has no existing row to scope against, so nothing narrows it.Repro
POST /v1/groupswith a group manager's access token →201.Also reachable through the UI:
/admin/groups/createrenders for a group manager (no route-level role check exists — see the companion issue), so the form submits successfully and redirects to the group list.Impact
Bounded, but real. The created group is instance-wide, is connected to every non-repo instrument, occupies the unique-name space, and appears in every admin's group list.
It is not a data-access escalation: permissions are frozen into the JWT at login, so the creator is not a member of the group they just created and cannot read anything through it.
Suggested fix
Either:
@RouteAccessonGroupsController.createto{ action: 'manage', subject: 'all' }, matching who the UI intends to expose it to and the siblingPATCH /v1/setup; orGroupsService.createan{ ability }parameter and check the payload withforcedAppSubject('Group', ...), the wayfiles.service.tsdoes.(1) is smaller and matches existing convention. Worth a unit test in
src/auth/__tests__/ability.factory.test.tsasserting the deny case either way.Notes
Found while writing the Playwright suite (branch
e2e-tests).testing/src/specs/authorization.spec.tsnow has aserver-side authorizationblock asserting the whole family of privileged requests is refused for non-admin roles;POST /v1/groupsis deliberately excluded from that table so the suite does not lock in this behaviour. Add it back when this is fixed.