From 9edbe142b6de8039c1d9ca0fa459e80a1f95fb79 Mon Sep 17 00:00:00 2001 From: Ali <22096341+alimuratgymn@users.noreply.github.com> Date: Sat, 25 Apr 2026 18:18:52 +0200 Subject: [PATCH] fix(tasks): prevent projectless task visibility leaks (#1066) Fixes task metrics queries that could show private projectless tasks in another user's Today/dashboard lists. The issue happened because dashboard-specific Op.or filters could overwrite the task visibility Op.or condition when query objects were combined with object spread. This addresses issue #1063 where tasks created from Inbox, Telegram, or directly in the web app could appear for other users when they were not assigned to a shared project. Changes: - Combined task visibility filters with dashboard filters using Op.and - Prevented metrics Op.or conditions from overwriting permission filters - Preserved access for owned, directly shared, and shared-project tasks - Added regression tests for tasks_in_progress and suggested_tasks leaks Fixes #1063 --- .../modules/tasks/queries/metrics-queries.js | 100 +++++++++++------- .../integration/permissions-tasks.test.js | 53 ++++++++++ 2 files changed, 114 insertions(+), 39 deletions(-) diff --git a/backend/modules/tasks/queries/metrics-queries.js b/backend/modules/tasks/queries/metrics-queries.js index ccb3881..d92b060 100644 --- a/backend/modules/tasks/queries/metrics-queries.js +++ b/backend/modules/tasks/queries/metrics-queries.js @@ -26,10 +26,14 @@ function isTaskInTodayPlan(task) { async function countTotalOpenTasks(visibleTasksWhere) { return await Task.count({ where: { - ...visibleTasksWhere, - status: { [Op.ne]: Task.STATUS.DONE }, - parent_task_id: null, - recurring_parent_id: null, + [Op.and]: [ + visibleTasksWhere, + { + status: { [Op.ne]: Task.STATUS.DONE }, + parent_task_id: null, + recurring_parent_id: null, + }, + ], }, raw: true, }); @@ -39,11 +43,15 @@ async function countTasksPendingOverMonth(visibleTasksWhere) { const oneMonthAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000); return await Task.count({ where: { - ...visibleTasksWhere, - status: { [Op.ne]: Task.STATUS.DONE }, - created_at: { [Op.lt]: oneMonthAgo }, - parent_task_id: null, - recurring_parent_id: null, + [Op.and]: [ + visibleTasksWhere, + { + status: { [Op.ne]: Task.STATUS.DONE }, + created_at: { [Op.lt]: oneMonthAgo }, + parent_task_id: null, + recurring_parent_id: null, + }, + ], }, raw: true, }); @@ -53,15 +61,23 @@ async function fetchTasksInProgress(visibleTasksWhere) { const now = new Date(); return await Task.findAll({ where: { - ...visibleTasksWhere, - status: { [Op.in]: [Task.STATUS.IN_PROGRESS, 'in_progress'] }, - // Exclude tasks deferred to the future - [Op.or]: [ - { defer_until: null }, - { defer_until: { [Op.lte]: now } }, + [Op.and]: [ + visibleTasksWhere, + { + status: { + [Op.in]: [Task.STATUS.IN_PROGRESS, 'in_progress'], + }, + parent_task_id: null, + recurring_parent_id: null, + }, + // Exclude tasks deferred to the future + { + [Op.or]: [ + { defer_until: null }, + { defer_until: { [Op.lte]: now } }, + ], + }, ], - parent_task_id: null, - recurring_parent_id: null, }, include: getTaskIncludeConfigLight(), order: [ @@ -280,15 +296,22 @@ async function fetchNonProjectTasks( limit = null ) { const exclusionIds = [...excludedTaskIds, ...somedayTaskIds]; + const filters = { + status: { + [Op.in]: [Task.STATUS.NOT_STARTED, Task.STATUS.WAITING], + }, + [Op.or]: [{ project_id: null }, { project_id: '' }], + parent_task_id: null, + recurring_parent_id: null, + }; + + if (exclusionIds.length > 0) { + filters.id = { [Op.notIn]: exclusionIds }; + } + const queryOptions = { where: { - ...visibleTasksWhere, - status: { - [Op.in]: [Task.STATUS.NOT_STARTED, Task.STATUS.WAITING], - }, - [Op.or]: [{ project_id: null }, { project_id: '' }], - parent_task_id: null, - recurring_parent_id: null, + [Op.and]: [visibleTasksWhere, filters], }, include: getTaskIncludeConfigLight(), order: [ @@ -298,10 +321,6 @@ async function fetchNonProjectTasks( ], }; - if (exclusionIds.length > 0) { - queryOptions.where.id = { [Op.notIn]: exclusionIds }; - } - if (limit && Number.isInteger(limit)) { queryOptions.limit = limit; } @@ -316,15 +335,22 @@ async function fetchProjectTasks( limit = null ) { const exclusionIds = [...excludedTaskIds, ...somedayTaskIds]; + const filters = { + status: { + [Op.in]: [Task.STATUS.NOT_STARTED, Task.STATUS.WAITING], + }, + project_id: { [Op.not]: null, [Op.ne]: '' }, + parent_task_id: null, + recurring_parent_id: null, + }; + + if (exclusionIds.length > 0) { + filters.id = { [Op.notIn]: exclusionIds }; + } + const queryOptions = { where: { - ...visibleTasksWhere, - status: { - [Op.in]: [Task.STATUS.NOT_STARTED, Task.STATUS.WAITING], - }, - project_id: { [Op.not]: null, [Op.ne]: '' }, - parent_task_id: null, - recurring_parent_id: null, + [Op.and]: [visibleTasksWhere, filters], }, include: getTaskIncludeConfigLight(), order: [ @@ -334,10 +360,6 @@ async function fetchProjectTasks( ], }; - if (exclusionIds.length > 0) { - queryOptions.where.id = { [Op.notIn]: exclusionIds }; - } - if (limit && Number.isInteger(limit)) { queryOptions.limit = limit; } diff --git a/backend/tests/integration/permissions-tasks.test.js b/backend/tests/integration/permissions-tasks.test.js index e863337..b7736e8 100644 --- a/backend/tests/integration/permissions-tasks.test.js +++ b/backend/tests/integration/permissions-tasks.test.js @@ -96,4 +96,57 @@ describe('Tasks Permissions', () => { expect(res.status).toBe(403); expect(res.body.error).toBe('Forbidden'); }); + + it("GET /api/tasks/metrics should not include another user's projectless in-progress task", async () => { + const myTask = await Task.create({ + name: 'My In Progress Task', + user_id: user.id, + status: Task.STATUS.IN_PROGRESS, + }); + const otherTask = await Task.create({ + name: 'Other In Progress Task', + user_id: otherUser.id, + status: Task.STATUS.IN_PROGRESS, + }); + + const res = await agent.get('/api/tasks/metrics'); + expect(res.status).toBe(200); + + const taskIds = res.body.tasks_in_progress.map((task) => task.id); + expect(taskIds).toContain(myTask.id); + expect(taskIds).not.toContain(otherTask.id); + }); + + it("GET /api/tasks/metrics should not suggest another user's projectless task", async () => { + await Task.bulkCreate([ + { + name: 'My Task 1', + user_id: user.id, + status: Task.STATUS.NOT_STARTED, + }, + { + name: 'My Task 2', + user_id: user.id, + status: Task.STATUS.NOT_STARTED, + }, + { + name: 'My Task 3', + user_id: user.id, + status: Task.STATUS.NOT_STARTED, + }, + ]); + const otherTask = await Task.create({ + name: 'Other Suggested Task', + user_id: otherUser.id, + status: Task.STATUS.NOT_STARTED, + }); + + const res = await agent.get('/api/tasks/metrics'); + expect(res.status).toBe(200); + + const suggestedTaskIds = res.body.suggested_tasks.map( + (task) => task.id + ); + expect(suggestedTaskIds).not.toContain(otherTask.id); + }); });