Handle unauthorized task detail deep links#1287
Conversation
Co-authored-by: Neil Raina <makeitraina@users.noreply.github.com>
Co-authored-by: Neil Raina <makeitraina@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
priosshrsth
left a comment
There was a problem hiding this comment.
@SandipBajracharya @arpandhakal This lgtm. What are your thoughts?
|
Deployment failed with the following error: Learn More: https://vercel.link/multiple-function-regions |
Greptile SummaryThis PR fixes an unhandled Sentry error (TASKS-90) that occurred when an internal user followed a deep link to a task they are not authorized to access. Previously only
Confidence Score: 4/5The loader change is narrow and correct — the 401 path is now handled consistently with 404, and the underlying auth guard is untouched. The one-line change in loadTask is well-targeted and the new tests confirm the 404/401/500 branches. The open question is whether loadTaskPath and loadSubtaskStatus would stay safe if access-control checks were later added to getTraversalPath or getSubtaskStatus, since those loaders still only catch 404. src/app/detail/[task_id]/[user_type]/loaders.ts — specifically the sibling loaders loadTaskPath and loadSubtaskStatus and whether their error guards should be kept in sync with loadTask. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Page as page.tsx
participant LT as loadTask
participant LTP as loadTaskPath
participant LS as loadSubtaskStatus
participant TS as TasksService.getOneTask
Browser->>Page: GET /detail/:task_id (deep link, unauthorized)
Page->>Page: Promise.all([loadTask, loadTaskPath, loadSubtaskStatus, loadViewSettings])
Page->>LT: loadTask(user, taskId)
LT->>TS: getOneTask(taskId)
TS->>TS: checkClientAccessForTask throws APIError(401)
TS-->>LT: throw APIError(401)
Note over LT: NEW: catches 401, returns null
LT-->>Page: null
Page->>LTP: loadTaskPath(user, taskId)
LTP-->>Page: [] (getTraversalPath has no access check)
Page->>LS: loadSubtaskStatus(user, taskId)
LS-->>Page: count 0 (getSubtaskStatus has no access check)
Page->>Page: "task === null, render DeletedRedirectPage"
Page-->>Browser: DeletedRedirectPage (no Sentry error)
|
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Changes
APIError401 responses fromloadTaskas a non-renderable task on the detail page, matching the existing 404 handling.TasksService.checkClientAccessForTaskauthorization guard so inaccessible tasks are still denied.Testing Criteria
yarn test --runTestsByPath 'src/app/detail/[task_id]/[user_type]/loaders.test.ts' --runInBandpasses and covers the unauthorized detail-load path from Sentry.yarn lint:check src/app/detail/[task_id]/[user_type]/loaders.ts src/app/detail/[task_id]/[user_type]/loaders.test.tsexits successfully (repo-wide warnings only).yarn tscwas run; it no longer reports errors from the changed loader code, but still fails on pre-existing missing SVG module declarations insrc/icons/index.ts.Notes
tasksproject, linked to Linear OUT-3831. The production event was an internal-user detail-page request for an inaccessible task from notification/deep-link traffic.auth_revokedduring this automation run, so I could not post the required ticket comment from the agent.Impact & Surface Area of Change