🛡️ Sentinel: Fix Mass Assignment in calendar notes#521
🛡️ Sentinel: Fix Mass Assignment in calendar notes#521ngoiyaeric wants to merge 1 commit intomainfrom
Conversation
Refactor the `saveNote` server action to explicitly list columns for `insert` and `update` operations. This prevents malicious users from overwriting protected fields like `userId` by providing unexpected keys in the JSON payload. - Removed object spreads (`...noteData`) in database calls. - Ensured `userId` is strictly taken from the authenticated session. - Added explicit mapping for allowed fields: `chatId`, `date`, `content`, `locationTags`, `userTags`, and `mapFeatureId`. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The mass-assignment mitigation is solid, but the update mapping may unintentionally overwrite columns with undefined if partial payloads are ever sent. Additionally, the insert path still appears to trust client-provided chatId without an explicit authorization check, which can enable cross-tenant associations depending on the data model. Addressing these will make the security fix more robust and prevent subtle data corruption.
Additional notes (1)
- Security |
lib/actions/calendar.ts:94-106
Insert usesuserId: userIdexplicitly (good), but the insert payload still directly trusts client-providedchatId. If notes are supposed to be scoped to chats the user belongs to, this allows a user to attach a note to an arbitrarychatIdthey can guess, which is a cross-tenant association risk unless there is an external authorization check elsewhere.
The update path does scope by userId, but the insert path doesn’t validate that the provided chatId belongs to the current user.
Summary of changes
What changed
- Added a short security note in
.jules/sentinel.mddocumenting a Mass Assignment / Overposting pattern and prevention guidance. - Hardened
saveNoteinlib/actions/calendar.tsby removing object spreads (...noteData) from Drizzle.set()and.values()calls. - Explicitly whitelisted persisted fields for both operations:
- Update:
chatId,date,content,locationTags,userTags,mapFeatureId, plusupdatedAt: new Date() - Insert:
userId(from session), and the same note fields
- Update:
- Preserved the update guard by scoping the update query to
where ... id = noteData.id AND userId = sessionUserId.
2 participants
This PR fixes a Mass Assignment (Overposting) vulnerability in the
saveNoteserver action withinlib/actions/calendar.ts. By explicitly listing the fields allowed for update and insert, we prevent potential data corruption or privilege escalation where a user could change the owner of a note or overwrite other protected fields.PR created automatically by Jules for task 12587019390571781091 started by @ngoiyaeric