fix(gateway): scope /api/teams endpoints to team membership (#1428) #1429

Merged
fred merged 1 commits from fix/teams-enumeration-scope into next 2026-08-26 22:45:55 +00:00
Collaborator

Closes #1428.

Problem (webUI audit A3 F9.2, upgraded to major in cross-review): every route in apps/gateway/src/workspace/teams.controller.ts sat behind AuthGuard only — any authenticated user could enumerate all teams and their full member records.

Fix:

  • GET /api/teams: non-admins get only their memberships via new TeamsService.findAllForUser (same two-step teamIds pattern as brain/projects.findAllForUser); admins keep findAll.
  • GET :teamId and :teamId/members: require admin or membership — 404 for a missing team, 403 for no access, the projects-controller convention.
  • GET :teamId/members/:userId: self-lookup stays open; looking up another user is team-scoped like the rest.

Compat: zero callers of /api/teams exist anywhere in the monorepo (grep across apps/packages/plugins), so no client changes.

Verification: new teams.controller.spec.ts (9 tests: member scoping, admin passthrough, 403/404 split, self-lookup, foreign-user denial). Gateway suite 818 passed / 77 files, typecheck clean, pre-push gates green.

Part of webui-audit S1 (lane REPORT.md); T10 ruled by operator 2026-08-26.

Closes #1428. **Problem (webUI audit A3 F9.2, upgraded to major in cross-review):** every route in `apps/gateway/src/workspace/teams.controller.ts` sat behind AuthGuard only — any authenticated user could enumerate all teams and their full member records. **Fix:** - `GET /api/teams`: non-admins get only their memberships via new `TeamsService.findAllForUser` (same two-step teamIds pattern as `brain/projects.findAllForUser`); admins keep `findAll`. - `GET :teamId` and `:teamId/members`: require admin or membership — 404 for a missing team, 403 for no access, the projects-controller convention. - `GET :teamId/members/:userId`: self-lookup stays open; looking up another user is team-scoped like the rest. **Compat:** zero callers of `/api/teams` exist anywhere in the monorepo (grep across apps/packages/plugins), so no client changes. **Verification:** new `teams.controller.spec.ts` (9 tests: member scoping, admin passthrough, 403/404 split, self-lookup, foreign-user denial). Gateway suite 818 passed / 77 files, typecheck clean, pre-push gates green. Part of webui-audit S1 (lane REPORT.md); T10 ruled by operator 2026-08-26.
fred added 1 commit 2026-08-26 22:17:07 +00:00
Every teams route sat behind AuthGuard only: any authenticated user
could enumerate all teams and their full member records (webUI audit
A3 F9.2, upgraded to major in cross-review).

- GET /api/teams: members see only their teams (new
  TeamsService.findAllForUser, same two-step pattern as
  brain/projects.findAllForUser); admins keep the full list.
- GET :teamId, :teamId/members: require admin or membership; 404 for a
  missing team, 403 for no access — the projects-controller convention.
- GET :teamId/members/:userId: self-lookup stays open; looking up
  another user is team-scoped like the rest.

No consumers of /api/teams exist in the monorepo, so no caller changes.
New controller spec: 9 tests. Gateway suite 818 passed, typecheck clean.
rev-code-01 approved these changes 2026-08-26 22:41:33 +00:00
rev-code-01 left a comment
Member

Code review by rev-code-01, pinned to dcb9da14 (author fred; Gate-16 clean). Independent verification of the lane's machine verdict (fleet/lanes/webui-audit/findings/pr1429-review.md) against the diff — claims re-checked by me, not relayed:

  • Controller scoping matches the fix description route by route: GET /api/teams admin→findAll / member→findAllForUser; GET :teamId and :teamId/members via getAccessibleTeam (404 missing / 403 non-member / admin passthrough, projects-controller convention); :teamId/members/:userId self-lookup open, foreign lookups gated. The gate runs before any member payload is fetched.
  • findAllForUser is the same two-step membership pattern as brain/projects (teamIds then inArray), empty-guard included, parameterized Drizzle — no injection surface.
  • Zero-caller compat claim re-verified by my own grep across apps/ packages/ plugins/: api/teams appears only in the controller and its new spec. No client breaks.
  • Fail-closed role handling confirmed: every check is exact === 'admin'; a missing role resolves to the member path, which strictly reduces access.
  • Tests run by me at this head: turbo run test typecheck --filter=@mosaicstack/gateway green — 818 passed / 45 skipped, 77 files, typecheck clean (matches the PR's claimed numbers exactly).
  • The lane review's residual notes (404/403 existence oracle = established repo convention; three optional spec gaps) are accurate and non-blocking.

CI 2801 was still running at review time; merge remains coordinator-gated and fred executes it, so terminal-green can be checked at merge time. Approving.

Code review by rev-code-01, pinned to dcb9da14 (author fred; Gate-16 clean). Independent verification of the lane's machine verdict (fleet/lanes/webui-audit/findings/pr1429-review.md) against the diff — claims re-checked by me, not relayed: - Controller scoping matches the fix description route by route: `GET /api/teams` admin→findAll / member→findAllForUser; `GET :teamId` and `:teamId/members` via getAccessibleTeam (404 missing / 403 non-member / admin passthrough, projects-controller convention); `:teamId/members/:userId` self-lookup open, foreign lookups gated. The gate runs before any member payload is fetched. - `findAllForUser` is the same two-step membership pattern as brain/projects (teamIds then inArray), empty-guard included, parameterized Drizzle — no injection surface. - Zero-caller compat claim re-verified by my own grep across apps/ packages/ plugins/: `api/teams` appears only in the controller and its new spec. No client breaks. - Fail-closed role handling confirmed: every check is exact `=== 'admin'`; a missing role resolves to the member path, which strictly reduces access. - Tests run by me at this head: `turbo run test typecheck --filter=@mosaicstack/gateway` green — 818 passed / 45 skipped, 77 files, typecheck clean (matches the PR's claimed numbers exactly). - The lane review's residual notes (404/403 existence oracle = established repo convention; three optional spec gaps) are accurate and non-blocking. CI 2801 was still running at review time; merge remains coordinator-gated and fred executes it, so terminal-green can be checked at merge time. Approving.
fred merged commit 49b7943420 into next 2026-08-26 22:45:55 +00:00
Sign in to join this conversation.