github-starred/actual
2
0
Fork 0
mirror of https://github.com/actualbudget/actual.git synced 2026-05-08 04:49:45 -05:00
Code Issues 1.6k Packages Projects Releases 55 Wiki Activity

[PR #6338] [MERGED] Fix authorization bypass allowing non-owners to delete shared budgets #60106

New Issue
Closed
opened 2026-05-07 01:58:43 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-07 01:58:43 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/actualbudget/actual/pull/6338
Author: @Copilot
Created: 12/8/2025
Status: ✅ Merged
Merged: 12/30/2025
Merged by: @MatissJanis

Base: master ← Head: copilot/fix-budget-deletion-permissions

📝 Commits (10+)

  • 5314b07 Initial plan
  • feb1ec9 Add permission checks for budget deletion
  • 03d8a45 Add release notes for PR #6338
  • da77c9a Update VRT screenshots
  • be51901 Fix: Change unauthorized to forbidden in delete-user-file
  • 7561683 Merge remote-tracking branch 'origin/master' into copilot/fix-budget-deletion-permissions
  • dec3bb5 Update VRT screenshots
  • 646760d Update VRT screenshots
  • d360265 Merge branch 'master' into copilot/fix-budget-deletion-permissions
  • ba8867a Fix: Update error reason from 'unauthorized' to 'forbidden' in delete-user-file response

📊 Changes

4 files changed (+171 additions, -50 deletions)

View changed files

📝 packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx (+65 -48)
📝 packages/sync-server/src/app-sync.test.ts (+82 -0)
📝 packages/sync-server/src/app-sync.ts (+18 -2)
➕ upcoming-release-notes/6338.md (+6 -0)

📄 Description

Description

Non-owner users could delete shared budgets from the sync server, causing the owner to lose access with unrecoverable "This file is not a cloud file" errors.

Changes

Server-side authorization (packages/sync-server/src/app-sync.ts)

  • Added permission check to /delete-user-file endpoint
  • Only file owners or server admins can delete from server
  • Returns 403 Forbidden for unauthorized attempts
const { user_id: userId } = res.locals;
const isOwner = file.owner === userId;
const isServerAdmin = isAdmin(userId);

if (!isOwner && !isServerAdmin) {
  res.status(403).send({
    status: 'error',
    reason: 'unauthorized',
    details: 'file-delete-not-allowed',
  });
  return;
}

Client-side protection (packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx)

  • Hides "Delete file from all devices" button for non-owners
  • Displays: "This is a hosted file shared with you. Only the file owner can delete it from the server."
  • Local deletion remains available to all users

Test coverage (packages/sync-server/src/app-sync.test.ts)

  • Non-owner receives 403
  • Owner can delete own files
  • Admin can delete any file
Original prompt

This section details on the original issue you should resolve

<issue_title>[Bug]: Non owner users able to delete shared budgets. And owners unable to register the budget on the sync error produced.</issue_title>
<issue_description>### Verified issue does not already exist?

  • I have searched and found no existing issue

What happened?

A bug happened!

Only Budget owner or server owner must have permission to delete a budget. All other shared budget users should not have delete access to the budget they don't own.

Image

How can we reproduce the issue?

How can we reproduce the issue?
Enabled multiuser support with OpenID with google.com

Server or Budget Owner creates a budget and shares it with another user.
The user deletes the budget from the switch files menu and thereby removing it from the server.
Now, for Server/budget owner, it shows a local only file.
Server/budget owner gets a sync error in their budget file and cannot recover the budget by trying to register (clicking the register button) from the UI.
Message:
"This file is not a cloud file
You need to register it to take advantage of syncing which allows you to use it across devices and never worry about losing your data. "

Where are you hosting Actual?

Pikapods

What browsers are you seeing the problem on?

Firefox, Chrome, Safari

Operating System

Windows 11</issue_description>

Comments on the Issue (you are @copilot in this section)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Note

Enforces authorization on POST /delete-user-file (owner/admin only), updates desktop modal to hide server delete for non-owners, adds tests, and notes in release docs.

  • Backend (sync-server):
    • Auth check: POST /delete-user-file now verifies requester is file owner or isAdmin(userId); otherwise returns 403 with reason: 'forbidden' and details: 'file-delete-not-allowed' in packages/sync-server/src/app-sync.ts.
    • Refactors file existence check to use returned file from verifyFileExists.
  • Frontend (desktop-client):
    • Ownership gating: DeleteFileModal reads state.user.data and shows server delete action only if file.owner === currentUserId or user has ADMIN permission; non-owners see explanatory text in packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx.
  • Tests:
    • Adds coverage for non-owner 403, owner success, and admin success in packages/sync-server/src/app-sync.test.ts.
  • Docs/Release notes:
    • Adds upcoming-release-notes/6338.md describing the bugfix.

Written by Cursor Bugbot for commit 034defddbe. This will update automatically on new commits. Configure here.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/actualbudget/actual/pull/6338 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 12/8/2025 **Status:** ✅ Merged **Merged:** 12/30/2025 **Merged by:** [@MatissJanis](https://github.com/MatissJanis) **Base:** `master` ← **Head:** `copilot/fix-budget-deletion-permissions` --- ### 📝 Commits (10+) - [`5314b07`](https://github.com/actualbudget/actual/commit/5314b0795dd8d8e30a8dfc77f3e7082d7f4818de) Initial plan - [`feb1ec9`](https://github.com/actualbudget/actual/commit/feb1ec98131704ca3d4bfe74641f64e6dfc16d35) Add permission checks for budget deletion - [`03d8a45`](https://github.com/actualbudget/actual/commit/03d8a45d96d3d9b2d5cf213b14e4ac56a9c3f557) Add release notes for PR #6338 - [`da77c9a`](https://github.com/actualbudget/actual/commit/da77c9a5a2d6c2dbb946b265875ad160dc996704) Update VRT screenshots - [`be51901`](https://github.com/actualbudget/actual/commit/be51901ed236b40d38bd243dbce463257d645f7a) Fix: Change unauthorized to forbidden in delete-user-file - [`7561683`](https://github.com/actualbudget/actual/commit/7561683f71b4a9930110b834cb2fe6e11565b170) Merge remote-tracking branch 'origin/master' into copilot/fix-budget-deletion-permissions - [`dec3bb5`](https://github.com/actualbudget/actual/commit/dec3bb5498da35018dc775490086616d0f8cd74a) Update VRT screenshots - [`646760d`](https://github.com/actualbudget/actual/commit/646760d1b5bb13d34cbb1458319ded92e647e8e8) Update VRT screenshots - [`d360265`](https://github.com/actualbudget/actual/commit/d360265a06436b0fe77ed376cc03705a3071a828) Merge branch 'master' into copilot/fix-budget-deletion-permissions - [`ba8867a`](https://github.com/actualbudget/actual/commit/ba8867a2185952bed5f49b6f2cbd4b37064c7678) Fix: Update error reason from 'unauthorized' to 'forbidden' in delete-user-file response ### 📊 Changes **4 files changed** (+171 additions, -50 deletions) <details> <summary>View changed files</summary> 📝 `packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx` (+65 -48) 📝 `packages/sync-server/src/app-sync.test.ts` (+82 -0) 📝 `packages/sync-server/src/app-sync.ts` (+18 -2) ➕ `upcoming-release-notes/6338.md` (+6 -0) </details> ### 📄 Description ## Description Non-owner users could delete shared budgets from the sync server, causing the owner to lose access with unrecoverable "This file is not a cloud file" errors. ### Changes **Server-side authorization** (`packages/sync-server/src/app-sync.ts`) - Added permission check to `/delete-user-file` endpoint - Only file owners or server admins can delete from server - Returns 403 Forbidden for unauthorized attempts ```typescript const { user_id: userId } = res.locals; const isOwner = file.owner === userId; const isServerAdmin = isAdmin(userId); if (!isOwner && !isServerAdmin) { res.status(403).send({ status: 'error', reason: 'unauthorized', details: 'file-delete-not-allowed', }); return; } ``` **Client-side protection** (`packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx`) - Hides "Delete file from all devices" button for non-owners - Displays: "This is a hosted file shared with you. Only the file owner can delete it from the server." - Local deletion remains available to all users **Test coverage** (`packages/sync-server/src/app-sync.test.ts`) - Non-owner receives 403 - Owner can delete own files - Admin can delete any file <!-- START COPILOT ORIGINAL PROMPT --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[Bug]: Non owner users able to delete shared budgets. And owners unable to register the budget on the sync error produced.</issue_title> > <issue_description>### Verified issue does not already exist? > > - [x] I have searched and found no existing issue > > ### What happened? > > A bug happened! > > Only Budget owner or server owner must have permission to delete a budget. All other shared budget users should not have delete access to the budget they don't own. > > <img width="1920" height="914" alt="Image" src="https://github.com/user-attachments/assets/e810ad93-5366-4b2e-b149-8cd264c2c2f8" /> > > > ### How can we reproduce the issue? > > How can we reproduce the issue? > Enabled multiuser support with OpenID with google.com > > Server or Budget Owner creates a budget and shares it with another user. > The user deletes the budget from the switch files menu and thereby removing it from the server. > Now, for Server/budget owner, it shows a local only file. > Server/budget owner gets a sync error in their budget file and cannot recover the budget by trying to register (clicking the register button) from the UI. > Message: > "This file is not a cloud file > You need to register it to take advantage of syncing which allows you to use it across devices and never worry about losing your data. <Register button>" > > ### Where are you hosting Actual? > > Pikapods > > ### What browsers are you seeing the problem on? > > Firefox, Chrome, Safari > > ### Operating System > > Windows 11</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes actualbudget/actual#6334 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Enforces authorization on `POST /delete-user-file` (owner/admin only), updates desktop modal to hide server delete for non-owners, adds tests, and notes in release docs. > > - **Backend (sync-server)**: > - **Auth check**: `POST /delete-user-file` now verifies requester is file owner or `isAdmin(userId)`; otherwise returns `403` with `reason: 'forbidden'` and `details: 'file-delete-not-allowed'` in `packages/sync-server/src/app-sync.ts`. > - Refactors file existence check to use returned `file` from `verifyFileExists`. > - **Frontend (desktop-client)**: > - **Ownership gating**: `DeleteFileModal` reads `state.user.data` and shows server delete action only if `file.owner === currentUserId` or user has `ADMIN` permission; non-owners see explanatory text in `packages/desktop-client/src/components/modals/manager/DeleteFileModal.tsx`. > - **Tests**: > - Adds coverage for non-owner `403`, owner success, and admin success in `packages/sync-server/src/app-sync.test.ts`. > - **Docs/Release notes**: > - Adds `upcoming-release-notes/6338.md` describing the bugfix. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 034defddbe52bbbcec87a842eab32912b1becde9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-07 01:58:43 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
AI generated
API
bank sync
budgeting
bug
can’t replicate
dependencies
docker
documentation
electron
experimental feature
feature
feedback
goal templates
good first issue
help wanted
importers
maintenance
needs info
needs testing
needs triage
needs votes
openid
payees
pull-request
Mirrored from GitHub Pull Request
 regression
reports
responsive
rules
schedules
server
✨ merged
split transactions
tech debt
theme
transaction import
transaction reconciliation
transactions
translations
upstream
user interface
✅ approved
wontfix
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#60106
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?