Skip to content

fix: make clients shared across all users while keeping work entries per-user#481

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1776238208-fix-shared-clients
Open

fix: make clients shared across all users while keeping work entries per-user#481
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1776238208-fix-shared-clients

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot commented Apr 15, 2026

Summary

Removes user_email filtering from all client queries so that clients are visible and accessible to all authenticated users. Work entries remain scoped to the individual user.

Root cause: All client SQL queries (SELECT, UPDATE, DELETE) included WHERE user_email = ?, making clients invisible across different user sessions.

Changes across 6 files:

  • clients.js — Removed user_email from WHERE clauses in GET (all/by-id), PUT, DELETE (all/by-id) queries
  • workEntries.js — Client validation on POST/PUT now checks WHERE id = ? instead of WHERE id = ? AND user_email = ?; error message simplified to 'Client not found'
  • reports.js — Client lookup queries in client report, CSV export, and PDF export no longer filter by user_email; work entry queries in reports still filter by user_email
  • Test files — Updated assertions to match new query parameters and error messages

All 161 tests pass (8 suites).

Review & Testing Checklist for Human

  • DELETE /api/clients/ is now globally destructive — previously deleted only the authenticated user's clients (WHERE user_email = ?), now runs DELETE FROM clients with no filter. Any authenticated user can delete all clients for everyone. Confirm this is acceptable or if this endpoint needs a guard/removal.
  • Any authenticated user can update or delete any client by ID — no ownership or role check remains. Verify the desired access model is truly "all users can mutate all clients."
  • Manual test: Log in as user A, create a client, log out, log in as user B → verify the client is visible. Create a work entry as user B → log back in as user A → verify user A does NOT see user B's work entry but DOES see the shared client.
  • Error message change: 'Client not found or does not belong to user''Client not found' in work entry routes. Check if any frontend code matches on the old error string.

Notes

  • The user_email column is retained in the clients table schema and in the INSERT statement — it still records who created the client, it's just no longer used for access filtering.
  • The INSERT into clients still writes user_email (for audit purposes); no schema migration needed.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/1cb8983a95dc4e448f150b0deada1438


Open with Devin

…per-user

Remove user_email filtering from client queries (GET, GET by ID, PUT,
DELETE) so clients are visible to all authenticated users. Update client
validation in work entries and reports routes to check client existence
without user_email scoping.

Work entries remain scoped to individual users via user_email filtering
in all work entry queries and report work entry queries.

Updated tests to match new shared client behavior.
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines 193 to +195
db.run(
'DELETE FROM clients WHERE user_email = ?',
[req.userEmail],
'DELETE FROM clients',
[],
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 DELETE /api/clients allows any user to wipe ALL clients and cascade-delete ALL work entries for ALL users

The DELETE / route in backend/src/routes/clients.js:193-194 was changed from DELETE FROM clients WHERE user_email = ? to DELETE FROM clients with no WHERE clause. This means any single authenticated user can delete every client in the entire database. Due to the ON DELETE CASCADE foreign key on work_entries.client_id (backend/src/database/init.js:64), this also destroys all work entries for every user in the system. The frontend exposes this via deleteAllClients() at frontend/src/api/client.ts:84-87. Previously the blast radius was limited to the calling user's own clients; now it's the entire dataset.

Prompt for agents
The DELETE / route on clients.js now runs an unscoped DELETE FROM clients, allowing any authenticated user to wipe all clients and cascade-delete all work entries for all users. This was previously scoped to the authenticated user via WHERE user_email = ?. In a shared-clients model, this bulk-delete endpoint is extremely dangerous because it affects all users' data. Consider one of: (1) removing the bulk-delete endpoint entirely, (2) restricting it to an admin role, or (3) at minimum keeping the user_email scope so a user can only delete clients they created. The cascade behavior defined in backend/src/database/init.js:64 means deleting clients also removes all associated work_entries.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

0 participants