RBAC Quick Wins¶

Status: Design approved — ready for implementation plan Date: 2026-04-24 Owner: Fabio Origin: Sub-project 1 of the remediation portfolio derived from docs/superpowers/audits/2026-04-24-rbac-audit.md.

Context¶

The RBAC audit (2026-04-24) surfaced 12 findings against the authorization model. The portfolio of follow-up work was decomposed into five sub-projects; this spec covers the first one — a single small PR that bundles every quick-win change. Three audit findings collapse into one bundle because they're the same flavor of work (small, surgical, RBAC-housekeeping) and a single review is cheaper than three:

§3.1 (🔴): The three @AggregateResponse() routes in invitations.controller.ts return DTOs that do not extend AggregateResponseDto, violating the documented semantic contract. The runtime safety-net assertion in FieldFilterInterceptor doesn't catch this because no top-level key collides with a scope name on the invitations entity.
§3.7 (🟡): scope_field_mappings is a database table populated by the seed but never read at runtime. The TypeScript constants in src/common/constants/scope-fields.ts are the actual source of truth for both the seed and the services that consume scope→field maps. The DB rows are dead weight that creates dual-source-of-truth confusion.
§3.9 (🟡): NEVER_WRITABLE_FIELDS is a hand-curated allow-list of four entries (id, createdAt, updatedAt, tenantId). Several other identity/system field names that should never appear in a request body are missing.

In addition to fixing those three findings, the spec tightens the @AggregateResponse() decorator's signature so that the extends AggregateResponseDto invariant is enforced at compile time rather than only by reviewer convention. This was the cheapest available alternative to the rejected runtime module-init walker (§3.8) and prevents §3.1 from recurring without adding any runtime cost.

Goals¶

Make the bug in §3.1 statically impossible to reintroduce by tightening the @AggregateResponse() type signature, then fix the bug as a one-time mechanical update.
Drop the scope_field_mappings table; collapse the source-of-truth stack so the TypeScript constants in scope-fields.ts are the single canonical input.
Extend NEVER_WRITABLE_FIELDS with the missing identity/system field names so FieldWriteGuard denies them with a specific error rather than the generic "unknown key" path.
Update all docs and source comments that mention ScopeFieldMapping / scope_field_mappings so the project's documentation reconciles with the new state in the same PR.

Non-Goals¶

The runtime module-init validator from §3.8 (rejected — convention + the new compile-time enforcement is judged sufficient).
The other audit findings (§3.2, §3.4, §3.5, §3.6, §3.10) — each owned by a separate sub-project.
Postgres RLS (§3.3) — explicitly deferred per the user's prioritization decision.
Automatic derivation of NEVER_WRITABLE_FIELDS from Prisma metadata — the human-curated list is preferred.
Any change to the FieldFilterInterceptor runtime aggregate-shape assertion. It still serves as a top-level-key collision check and remains valuable.
Any change to controllers' permission decorator topology beyond updating @AggregateResponse() call sites.

Design¶

Compile-time enforcement of `AggregateResponseDto` extension¶

The decorator changes from a zero-arg flag to a one-arg form that takes the response DTO class. The argument is typed Type<T extends AggregateResponseDto>, so any class that does not extend the base class fails to typecheck at the call site.

Before:

export const AggregateResponse = () => SetMetadata(AGGREGATE_RESPONSE_KEY, true);

After:

import type { Type } from '@nestjs/common';
import { AggregateResponseDto } from '../../common/dto/aggregate-response.dto';

export const AggregateResponse =
  <T extends AggregateResponseDto>(dto: Type<T>) =>
    SetMetadata(AGGREGATE_RESPONSE_KEY, dto);

The metadata payload changes from boolean → Type<AggregateResponseDto>. FieldFilterInterceptor only uses the metadata as a presence flag (if (aggregate)), and a class object is truthy, so no interceptor logic changes. The interceptor's existing runtime collision assertion is preserved untouched.

The decorator's existing inline documentation block is updated to reflect that the extends AggregateResponseDto rule is now compile-time enforced rather than reviewer convention. The "for AI agents" caveat about not applying the marker as a convenient fix is preserved — the type system enforces the extension, but it cannot judge whether the response is actually free of scope-grouped semantics.

Bug fix §3.1: invitations DTOs¶

Two classes change:

src/invitations/dto/send-batch-result.dto.ts — SendBatchResultDto extends AggregateResponseDto
src/invitations/dto/projected-invitation.dto.ts — BaseProjectedInvitationDto extends AggregateResponseDto. The three concrete subclasses (TeacherInvitationDto, StaffInvitationDto, ReferentInvitationDto) inherit through the chain.

The discriminated-union type alias ProjectedInvitationDto is not a value and cannot be passed to the decorator. The two affected routes (GET /invitations, POST /invitations/:id/resend) pass BaseProjectedInvitationDto to @AggregateResponse(...) instead — the decorator's job is to statically prove the extension, not to model the discriminated union for Swagger.

Updated `@AggregateResponse()` call sites¶

All existing call sites get the response DTO class added as an argument. This is a single mechanical change per site and is the breaking-change cost of the decorator signature update.

File	Routes
`src/students/students.controller.ts`	`POST /students/import` → `@AggregateResponse(StudentImportSummaryDto)`
`src/teachers/teachers.controller.ts`	`POST /teachers/import` → `@AggregateResponse(TeacherImportSummaryDto)`
`src/staff/staff.controller.ts`	`POST /staff/import` → `@AggregateResponse(StaffImportSummaryDto)`
`src/invitations/invitations.controller.ts`	`POST /invitations/send` → `@AggregateResponse(SendBatchResultDto)`
`src/invitations/invitations.controller.ts`	`GET /invitations` → `@AggregateResponse(BaseProjectedInvitationDto)`
`src/invitations/invitations.controller.ts`	`POST /invitations/:id/resend` → `@AggregateResponse(BaseProjectedInvitationDto)`
`src/academic-years/academic-years.controller.ts`	`GET /academic-years` → `@AggregateResponse(AcademicYearListItemDto)`
`src/rooms/rooms.controller.ts`	`GET /rooms/types` → `@AggregateResponse(RoomTypeListItemDto)`
`src/curriculum/curriculum.controller.ts`	`GET /curricula/presets` → `@AggregateResponse(CurriculumPresetSummaryDto)`

All six DTOs verified to already extend AggregateResponseDto — the three import summaries do so via the ImportSummaryDtoOf(...) mixin in src/common/dto/import-summary.dto.ts; the three lookup DTOs do so directly. Each route just needs the decorator argument added.

After this change, any future @AggregateResponse(NotABaseSubclass) fails tsc.

§3.7: drop `scope_field_mappings`¶

The table is removed end-to-end:

Schema (prisma/schema.prisma): delete the ScopeFieldMapping model and the fieldMappings relation on PermissionScope.
Migration: a new migration DROP TABLE scope_field_mappings. The hazard checklist in docs/12-migrations.md is followed; this is the safest possible migration shape (drop on a never-read table). No down migration is written, consistent with project convention.
Seed (prisma/seed/rbac-catalogue.ts): remove the FIELD_MAPPINGS constant and its loop body. The RbacCatalogueResult interface (lines 164–169) does not currently expose field mappings, so its shape stays unchanged. The seed continues to populate entities, scopes, actions, action-scope-requirements, and others-scopes — only the field-mapping rows go away.
Source of truth: STUDENT_SCOPES, TEACHER_SCOPES, etc. in src/common/constants/scope-fields.ts remain unchanged. They continue to feed BaseTenantedCrudService.getScopeFieldMappings() (consumed by services) and ENTITY_SCOPE_REGISTRY (consumed by FieldFilterInterceptor's aggregate-shape assertion).
Documentation: chapter 04 sections describing the table are removed or replaced — see "Documentation updates" below.

The simplification of the source-of-truth stack:

Before:                              After:
scope-fields.ts                      scope-fields.ts
    │                                    │
    ├→ seed → scope_field_mappings       ├→ ENTITY_SCOPE_REGISTRY (interceptor)
    │       (DB table, unused)           └→ getScopeFieldMappings() (base service)
    ├→ ENTITY_SCOPE_REGISTRY
    └→ getScopeFieldMappings()

Three layers collapse to two with no information loss.

§3.9: extend `NEVER_WRITABLE_FIELDS`¶

src/permissions/constants/system-fields.ts:

export const NEVER_WRITABLE_FIELDS = new Set([
  'id', 'createdAt', 'updatedAt', 'tenantId',
  // Identity/system fields added 2026-04-24 (audit §3.9)
  'personUuid', 'userId', 'passwordHash',
  'isPlatformAdmin', 'isActive',
]);

FieldWriteGuard's "unknown top-level key" check already rejects these names because none of them are scope keys. The explicit deny-list provides defense in depth and a more specific log message ("Forbidden field write: passwordHash for user X") than the generic unknown-key rejection. No behavior change for valid requests.

Documentation updates¶

References to ScopeFieldMapping / scope_field_mappings exist across multiple docs and one source comment. All must be reconciled in the same PR — leaving stale references is exactly the dual-source-of-truth problem this work is meant to eliminate.

docs/04-rbac.md (primary RBAC chapter):

§"Mental Model" line 71 — "Each scope maps to specific database fields via scope_field_mappings" → reword as "via the field arrays in src/common/constants/scope-fields.ts".
§"Database Schema" — remove the scope_field_mappings row from the system-level table and the entire #### scope_field_mappings subsection. Also remove the "Important: Field names MUST match the keys returned by Prisma queries" callout (the concern survives via getScopeFieldMappings() matching real Prisma keys, which is already enforced by the BaseTenantedCrudService contract — note this in §"Permission Seed Patterns" instead).
§"Recipe: scope spanning multiple tables" line 473 — update the explanation that mentions scope_field_mappings as the physical layer. The multi-table case now works because both getScopeFieldMappings() and the seed's per-table loop accept the same flat field arrays from scope-fields.ts.
§"New Entity Permission Checklist" line 517 — remove item 4 ("ScopeFieldMapping rows in seed FIELD_MAPPINGS").
§"Shared Scope Field Constants" lines 738, 753 — remove the parenthetical "for ScopeFieldMapping rows" and the sentence about "the seed ScopeFieldMapping rows pick it up automatically".
§"Aggregate responses" inside §"Interceptor Contract: Scope-Grouped vs Aggregate Responses" — update the decorator example to use the one-arg form. Replace the runtime-only enforcement language with an explicit note that the extends AggregateResponseDto rule is compile-time enforced by the decorator's type signature, while the runtime collision assertion remains as an additional safety net for top-level key bugs.
§"Rules for applying @AggregateResponse()" — preserve the security contract list. Update Rule 1 ("response DTO must extend AggregateResponseDto") to note this is now tsc-enforced.

docs/REFERENCE.md:

Line 100 (RBAC invariants) — "New scopes require a PermissionScope + ScopeFieldMapping seed entry, an array in src/common/constants/scope-fields.ts, and a matching DTO" → drop the ScopeFieldMapping mention; the PermissionScope row + the constant array + the DTO are sufficient.
Line 135 (file index for "Add a new scope") — file list stays correct; the prisma/seed/rbac-catalogue.ts entry no longer needs a FIELD_MAPPINGS edit, but is still touched for the SCOPES entry. No change needed unless we want to be explicit.

docs/11-workflows.md:

§1 ("Add a field to an existing scope"), lines 22, 24 — drop the "ScopeFieldMapping rows in seed use the shared constant arrays" sentence; the workflow becomes "add field to constant array; if function-mapped, also update getScopeFieldMappings()".
§2 ("Add a scope to an entity"), lines 41, 42 — drop the ScopeFieldMapping mention from step 9; the seed step is now PermissionScope only (plus ActionScopeRequirement if the scope is required by an action).
§3 ("Add a new entity"), lines 91, 99 — drop ScopeFieldMapping rows from the rbac-catalogue.ts checklist.

src/common/constants/scope-fields.ts (file header comment, lines 4 and 7):

"and seed (ScopeFieldMapping rows)" → "and seed (PermissionScope rows)"
"Both the service's getScopeFieldMappings() and the seed pick it up automatically" → unchanged (still accurate; both still consume the constants).

Decorator inline docs (src/permissions/decorators/aggregate-response.decorator.ts):

The "Required: extend AggregateResponseDto" section becomes a statement that the rule is enforced by the decorator's type parameter — tsc rejects non-conforming classes at the call site. The "Runtime safety net" paragraph stays (it serves the orthogonal purpose of catching scope-name collisions in aggregate response shapes).
The "Security contract" rule numbered "the response DTO must extend AggregateResponseDto" is updated to note this is now compile-time enforced.
The "for AI agents" caveat stays unchanged — the type system enforces extension, but cannot judge whether the response is actually free of scope-grouped semantics.

docs/05-crud-patterns.md and docs/13-typing-conventions.md only reference getScopeFieldMappings() (the service method), which is unaffected. No changes needed there.

Testing¶

Existing unit tests (field-filter.interceptor.spec.ts, field-write.guard.spec.ts, all controller specs) should pass unchanged. The metadata-payload type change from boolean to Type is transparent to the interceptor's truthiness check.
Negative compile test for the decorator: a single // @ts-expect-error line in a new test file (e.g. src/permissions/decorators/aggregate-response.decorator.spec-types.ts) demonstrates that @AggregateResponse(class NotExtending {}) fails to typecheck. Locks the contract test-style and ensures it never silently regresses.
field-write.guard.spec.ts — add cases asserting that body keys passwordHash, isPlatformAdmin, userId produce 403 FORBIDDEN_FIELDS with the system-field log path (rather than the unknown-key path).
Migration smoke: rerun npx prisma migrate dev then npx prisma db seed end-to-end. No errors. scope_field_mappings is gone from the DB.

No new test infrastructure required.

Risks and rollback¶

Decorator signature is a breaking change to internal callers. Mitigated by updating all 9 call sites in the same PR (atomic). No external consumers — the decorator is a project-internal utility.
DROP TABLE scope_field_mappings migration is the safest possible shape — a never-read table on tenants that have only ever been seeded. Beta tenants only; no production data to lose. The wipe-and-reseed posture confirmed for the broader portfolio means tenants can be reseeded if anything looks off.
Rollback is whole-PR revert + manual re-creation of the table via a forward migration if (improbably) needed. Acceptable given nothing reads from the table.

Order of operations within the PR¶

The order minimizes cross-step surprise — schema/seed first (so a clean reseed validates the catalogue), then decorator changes, then docs.

Drop scope_field_mappings: schema, migration, seed cleanup.
Reseed locally; verify the catalogue still produces the expected entities/scopes/actions.
Extend NEVER_WRITABLE_FIELDS; add the new test cases to field-write.guard.spec.ts.
Bug fix §3.1: extend AggregateResponseDto on SendBatchResultDto and BaseProjectedInvitationDto.
Decorator signature + payload change in aggregate-response.decorator.ts.
Update all 9 @AggregateResponse(...) call sites to pass the DTO class.
Add the negative compile test for the decorator.
Documentation sweep: docs/04-rbac.md, docs/REFERENCE.md, docs/11-workflows.md, the file-header comment on scope-fields.ts, and the inline docs on aggregate-response.decorator.ts — all in the same PR per the "Documentation updates" section above.
npm run build && npm test (user runs).

Open questions¶

None. All choices are settled per the brainstorm:

A2 chosen over A1 — compile-time enforcement preferred over surgical-only fix.
B1 chosen over B2 — bundle quick-win sub-projects into one PR rather than three.
§3.9 stays human-curated rather than auto-derived from Prisma metadata.
§3.7 drops the table entirely; the Option C structural-collapse refactor is deferred as not-needed.