fix(gateway): enforce task and mission ownership
This commit is contained in:
@@ -86,4 +86,52 @@ describe('Resource ownership checks', () => {
|
||||
ForbiddenException,
|
||||
);
|
||||
});
|
||||
|
||||
it('forbids creating a task with an unowned project', async () => {
|
||||
const brain = createBrain();
|
||||
brain.projects.findById.mockResolvedValue({ id: 'project-1', ownerId: 'user-2' });
|
||||
const controller = new TasksController(brain as never);
|
||||
|
||||
await expect(
|
||||
controller.create(
|
||||
{
|
||||
title: 'Task',
|
||||
projectId: 'project-1',
|
||||
},
|
||||
{ id: 'user-1' },
|
||||
),
|
||||
).rejects.toBeInstanceOf(ForbiddenException);
|
||||
});
|
||||
|
||||
it('forbids listing tasks for an unowned project', async () => {
|
||||
const brain = createBrain();
|
||||
brain.projects.findById.mockResolvedValue({ id: 'project-1', ownerId: 'user-2' });
|
||||
const controller = new TasksController(brain as never);
|
||||
|
||||
await expect(
|
||||
controller.list({ id: 'user-1' }, 'project-1', undefined, undefined),
|
||||
).rejects.toBeInstanceOf(ForbiddenException);
|
||||
});
|
||||
|
||||
it('lists only tasks for the current user owned projects when no filter is provided', async () => {
|
||||
const brain = createBrain();
|
||||
brain.projects.findAll.mockResolvedValue([
|
||||
{ id: 'project-1', ownerId: 'user-1' },
|
||||
{ id: 'project-2', ownerId: 'user-2' },
|
||||
]);
|
||||
brain.missions.findAll.mockResolvedValue([{ id: 'mission-1', projectId: 'project-1' }]);
|
||||
brain.tasks.findAll.mockResolvedValue([
|
||||
{ id: 'task-1', projectId: 'project-1' },
|
||||
{ id: 'task-2', missionId: 'mission-1' },
|
||||
{ id: 'task-3', projectId: 'project-2' },
|
||||
]);
|
||||
const controller = new TasksController(brain as never);
|
||||
|
||||
await expect(
|
||||
controller.list({ id: 'user-1' }, undefined, undefined, undefined),
|
||||
).resolves.toEqual([
|
||||
{ id: 'task-1', projectId: 'project-1' },
|
||||
{ id: 'task-2', missionId: 'mission-1' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -36,7 +36,10 @@ export class MissionsController {
|
||||
}
|
||||
|
||||
@Post()
|
||||
async create(@Body() dto: CreateMissionDto) {
|
||||
async create(@Body() dto: CreateMissionDto, @CurrentUser() user: { id: string }) {
|
||||
if (dto.projectId) {
|
||||
await this.getOwnedProject(dto.projectId, user.id, 'Mission');
|
||||
}
|
||||
return this.brain.missions.create({
|
||||
name: dto.name,
|
||||
description: dto.description,
|
||||
|
||||
@@ -28,17 +28,48 @@ export class TasksController {
|
||||
|
||||
@Get()
|
||||
async list(
|
||||
@CurrentUser() user: { id: string },
|
||||
@Query('projectId') projectId?: string,
|
||||
@Query('missionId') missionId?: string,
|
||||
@Query('status') status?: string,
|
||||
) {
|
||||
if (projectId) return this.brain.tasks.findByProject(projectId);
|
||||
if (missionId) return this.brain.tasks.findByMission(missionId);
|
||||
if (status)
|
||||
return this.brain.tasks.findByStatus(
|
||||
status as Parameters<typeof this.brain.tasks.findByStatus>[0],
|
||||
);
|
||||
return this.brain.tasks.findAll();
|
||||
if (projectId) {
|
||||
await this.getOwnedProject(projectId, user.id, 'Task');
|
||||
return this.brain.tasks.findByProject(projectId);
|
||||
}
|
||||
if (missionId) {
|
||||
await this.getOwnedMission(missionId, user.id, 'Task');
|
||||
return this.brain.tasks.findByMission(missionId);
|
||||
}
|
||||
|
||||
const [projects, missions, tasks] = await Promise.all([
|
||||
this.brain.projects.findAll(),
|
||||
this.brain.missions.findAll(),
|
||||
status
|
||||
? this.brain.tasks.findByStatus(
|
||||
status as Parameters<typeof this.brain.tasks.findByStatus>[0],
|
||||
)
|
||||
: this.brain.tasks.findAll(),
|
||||
]);
|
||||
|
||||
const ownedProjectIds = new Set(
|
||||
projects.filter((project) => project.ownerId === user.id).map((project) => project.id),
|
||||
);
|
||||
const ownedMissionIds = new Set(
|
||||
missions
|
||||
.filter(
|
||||
(ownedMission) =>
|
||||
typeof ownedMission.projectId === 'string' &&
|
||||
ownedProjectIds.has(ownedMission.projectId),
|
||||
)
|
||||
.map((ownedMission) => ownedMission.id),
|
||||
);
|
||||
|
||||
return tasks.filter(
|
||||
(task) =>
|
||||
(task.projectId ? ownedProjectIds.has(task.projectId) : false) ||
|
||||
(task.missionId ? ownedMissionIds.has(task.missionId) : false),
|
||||
);
|
||||
}
|
||||
|
||||
@Get(':id')
|
||||
@@ -47,7 +78,13 @@ export class TasksController {
|
||||
}
|
||||
|
||||
@Post()
|
||||
async create(@Body() dto: CreateTaskDto) {
|
||||
async create(@Body() dto: CreateTaskDto, @CurrentUser() user: { id: string }) {
|
||||
if (dto.projectId) {
|
||||
await this.getOwnedProject(dto.projectId, user.id, 'Task');
|
||||
}
|
||||
if (dto.missionId) {
|
||||
await this.getOwnedMission(dto.missionId, user.id, 'Task');
|
||||
}
|
||||
return this.brain.tasks.create({
|
||||
title: dto.title,
|
||||
description: dto.description,
|
||||
|
||||
58
docs/scratchpads/task-mission-ownership-20260313.md
Normal file
58
docs/scratchpads/task-mission-ownership-20260313.md
Normal file
@@ -0,0 +1,58 @@
|
||||
# Task Ownership Gap Fix Scratchpad
|
||||
|
||||
## Metadata
|
||||
|
||||
- Date: 2026-03-13
|
||||
- Worktree: `/home/jwoltje/src/mosaic-mono-v1-worktrees/fix-task-ownership`
|
||||
- Branch: `fix/task-mission-ownership`
|
||||
- Scope: Fix ownership checks in TasksController/MissionsController and extend gateway ownership tests
|
||||
- Related tracker: worker task only; `docs/TASKS.md` is orchestrator-owned and left unchanged
|
||||
- Budget assumption: no explicit token cap; keep scope limited to requested gateway permission fixes
|
||||
|
||||
## Objective
|
||||
|
||||
Close ownership gaps so task listing/creation and mission creation enforce project/mission ownership and reject cross-user access.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
1. TasksController `list()` enforces ownership for `projectId` and `missionId`, and does not return cross-user data when neither filter is provided.
|
||||
2. TasksController `create()` rejects unowned `projectId` and `missionId` references.
|
||||
3. MissionsController `create()` rejects unowned `projectId` references.
|
||||
4. Gateway ownership tests cover forbidden task creation and forbidden task listing by unowned project.
|
||||
|
||||
## Plan
|
||||
|
||||
1. Inspect current controller and ownership test patterns.
|
||||
2. Add failing permission tests first.
|
||||
3. Patch controller methods with existing ownership helpers.
|
||||
4. Run targeted gateway tests, then gateway typecheck/lint/full test.
|
||||
5. Perform independent review, record evidence, then complete the requested git/PR workflow.
|
||||
|
||||
## TDD Notes
|
||||
|
||||
- Required: yes. This is auth/permission logic and a bugfix.
|
||||
- Strategy: add failing tests in `resource-ownership.test.ts`, verify red, then implement minimal controller changes.
|
||||
|
||||
## Verification Log
|
||||
|
||||
- `pnpm --filter @mosaic/gateway test -- src/__tests__/resource-ownership.test.ts`
|
||||
- Red: failed with 2 expected permission-path failures before controller changes.
|
||||
- Green: passed after wiring ownership checks and adding owned-task filtering coverage.
|
||||
- `pnpm --filter @mosaic/gateway typecheck`
|
||||
- Pass on 2026-03-13 after fixing parameter ordering and mission project nullability.
|
||||
- `pnpm --filter @mosaic/gateway lint`
|
||||
- Pass on 2026-03-13.
|
||||
- `pnpm --filter @mosaic/gateway test`
|
||||
- Pass on 2026-03-13 with 3 test files and 23 tests passing.
|
||||
- `pnpm format:check`
|
||||
- Pass on 2026-03-13.
|
||||
|
||||
## Review Log
|
||||
|
||||
- Manual review: checked for auth regressions, cross-user list leakage, and dashboard behavior impact; kept unfiltered task list functional by filtering to owned projects/missions instead of returning an empty list.
|
||||
- Automated review: `~/.config/mosaic/tools/codex/codex-code-review.sh --uncommitted` running/re-run for independent review evidence.
|
||||
|
||||
## Risks / Blockers
|
||||
|
||||
- Repository-wide Mosaic instructions require merge/issue closure, but the user explicitly instructed PR-only and no merge; follow the user instruction.
|
||||
- `docs/TASKS.md` is orchestrator-owned and will not be edited from this worker task.
|
||||
Reference in New Issue
Block a user