Skip to content

Check on the server who may change what (writes) - #559

Merged
doomlab merged 2 commits into
fix/539-restrict-task-log-accessfrom
fix/server-side-access-checks
Oct 9, 2026
Merged

doomlab merged 2 commits into
fix/539-restrict-task-log-accessfrom
fix/server-side-access-checks

Conversation

@doomlab

@doomlab doomlab commented Oct 8, 2026

Copy link
Copy Markdown
Member

Stage 1 of closing the server-side access gap found while tracing #539. Stacked on #558 (it reuses its access helper), so this targets that branch; once #558 merges, retarget to dev. Please review the rules below: this is a behaviour change on the server.

Problem

Most write endpoints only checked that the caller was signed in. Hiding a button in the UI is not protection: anyone signed in could send the request directly. For example deleteTask({ id }) deleted the task and its logs for anybody (the file even had a TODO: add validation to ensure correct tenant), and updateProjectMember took privilege, projectId and userId from the browser, so a contributor could promote themselves to project manager. Task ids are small sequential numbers, so they are easy to guess.

What changed

New requireAccess.ts (next to getProjectAccess) with checks that report anything not allowed as "not found". Applied to 50 endpoints:

Endpoints Who may
Projects (update, delete, copy), milestones, columns, tasks (create, update, delete, status, order, dates, roles, milestone links), teams, member roles, contributors (delete), invites (create), announcements, project-manager widgets Project managers of that project. Anything they attach (members, columns, milestones, tasks) must be in the same project.
Task log updates and comments The people the log is assigned to, or the project's managers; the "who is doing this" member must be the caller. Approval: managers only.
Accept / decline an invitation The person it is for (matched by email, as the invites list does). Managers can also decline/manage their project's invitations, but not accept for someone.
Roles The person who made it, or the managers of its project. A browser-supplied userId can no longer reassign a role.
Forms, form versions, folders Their owner.
createForm, importDefaultRoles, createRole, initializeWidgets Only for yourself (the browser-named userId must be the caller).
initializeProjectWidgets A manager, or the person themselves.
comments.markAsRead Had no sign-in check at all; now requires one and that the member is yours.
The five one-off src/updates/queries/* migrations Administrators only on the server. They were callable by any signed-in user (the dashboard one deleted and rebuilt every user's widgets); only the page hiding them was admin-only.

createDefaultWidgets was split out of initializeWidgets so the admin migration doesn't call the now self-only mutation.

Not covered (follow-ups)

  • Read queries that trust a filter from the browser (getComments, getTasks, getTask, getMilestones, getRoles, getTags, getProjectMembers, getUsers, ...). This PR is writes only.
  • sendNotification: other mutations call it internally, so it needs to be split into a safe internal function first. It can currently be used to send notifications to arbitrary users.
  • Attaching a role to a task/member and a form version to a task is not checked against ownership.

Testing

  • mutationAccess.test.ts: every one of 28 write endpoints is called as (a) a signed-in user with no part in the project and (b) a plain contributor, against a database stand-in that records writes. All must refuse with "not found" and write nothing. The same 28 as a project manager must get past the check. All 56 refusal tests fail against the old code and pass now.
  • requireAccess.test.ts (14 tests) for each rule (invite email match, self-only, form/folder/role ownership, same-project attachments, ...), plus tests for initializeWidgets and the admin-only updates (fail against the old code).
  • Typecheck (apart from the existing getJsonSchema.test.ts errors), lint and the full suite (209 tests) pass.
  • Not yet checked in the app: please try as a project manager and as a contributor (create/edit/delete a task, drag tasks on the board, edit a team, invite someone, accept an invite, submit and comment on a task, edit and delete a form and a folder).

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant