fix: validate upload ownership in sendFileMessage

[sahillllllllll-bit]

Proposed changes (including videos or screenshots)

This PR fixes an authorization flaw in the file message flow.

The method sendFileMessage trusted the client-supplied file._id and immediately finalized the upload using Uploads.updateFileComplete.
The server did not validate whether the upload document actually belonged to the requesting user or was associated with the target room.

Because _id is attacker-controlled input, an authenticated user could provide the _id of an existing upload and attach another user’s file to a room they had access to.

Fix

The server now validates ownership and room association before completing the upload:

• Fetch upload document from database using _id
• Verify upload.userId === currentUser._id
• Verify upload.rid === roomId

If validation fails, the method throws error-invalid-file.

This prevents unauthorized file finalization and protects upload ownership integrity.

---

Issue(s)

Closes: #38892

---

Steps to test or reproduce

1. Create two users (User A and User B)
2. Login as User A and upload a file in any room
3. Capture the upload _id from the network request (/ufs/ upload response)
4. Login as User B
5. Call sendFileMessage (or intercept the client request) and provide User A’s upload _id

Before fix

The file is successfully attached to the room even though it belongs to User A.

After fix

The server rejects the request and returns error-invalid-file.

---

Further comments

This is an example of an Insecure Direct Object Reference (IDOR) vulnerability.
The server relied on a client-provided identifier without verifying resource ownership.

The added validation ensures that uploads can only be finalized by their original uploader and only in the room they were uploaded for.

The change only introduces authorization checks and does not modify the normal upload workflow for valid users.

Summary by CodeRabbit

• Bug Fixes
  • Added validation to ensure uploaded files belong to the requesting user and are associated with the target room, preventing unauthorized file sharing and improving security.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9cea90e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3145c41 and 9cea90e.

📒 Files selected for processing (1)

• apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

• apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

🧠 Learnings (3)

📓 Common learnings

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 32703
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:52-58
Timestamp: 2026-02-12T15:39:28.416Z
Learning: In `apps/meteor/client/lib/chats/flows/uploadFiles.ts`, when E2E encryption is required but not allowed (e.g., `E2E_Enable_Encrypt_Files` setting is disabled), the function intentionally abandons the entire upload queue and displays a toast error. This fail-fast behavior prevents partial uploads when encryption requirements cannot be met and is the expected behavior, not a bug.

📚 Learning: 2026-02-12T15:39:28.416Z

Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 32703
File: apps/meteor/client/lib/chats/flows/uploadFiles.ts:52-58
Timestamp: 2026-02-12T15:39:28.416Z
Learning: In `apps/meteor/client/lib/chats/flows/uploadFiles.ts`, when E2E encryption is required but not allowed (e.g., `E2E_Enable_Encrypt_Files` setting is disabled), the function intentionally abandons the entire upload queue and displays a toast error. This fail-fast behavior prevents partial uploads when encryption requirements cannot be met and is the expected behavior, not a bug.

Applied to files:

• apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

📚 Learning: 2025-11-04T16:49:19.107Z

Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.

Applied to files:

• apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

🔇 Additional comments (1)

apps/meteor/app/file-upload/server/methods/sendFileMessage.ts (1)

41-47: Ownership validation is correct — no app-user exemption or exported function issues found.

The fix properly validates file ownership by fetching the upload server-side and gating on upload.userId === user._id before mutation. Verification confirms the ownership check will not break legitimate use cases:

• App user flow: Apps creating files via AppUploadBridge.createUpload ensure upload.userId matches the userId passed to sendFileMessage, so the check passes.
• Exported function usage: parseFileIntoMessageAttachments is re-exported via the upload service wrapper, but actual callers (Meteor method, app bridge) all maintain consistent user identity between file creation and message sending.
• No system proxying: No code path found where a system/admin process sends files on behalf of a different user, so the ownership check won't incorrectly reject valid requests.

The validation is solid and requires no additional exemptions.

---

Walkthrough

Adds upload ownership and room association validation to the sendFileMessage method. Before processing a file, the server now verifies that the upload exists, belongs to the requesting user, and is associated with the target room, preventing unauthorized file finalization.

Changes

Cohort / File(s)	Summary
Upload Ownership Validation apps/meteor/app/file-upload/server/methods/sendFileMessage.ts	Added validation to verify uploaded file ownership and room association before processing. Checks upload existence, user ownership (upload.userId matches user._id), and room association (upload.rid matches roomId), throwing an error if any verification fails.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit guards each file with pride,
No stranger's uploads slip inside,
With checks so thorough, wise, and true,
Each file verified through and through!
Security hops along the way,
Protecting uploads every day! 🔒

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: validate upload ownership in sendFileMessage' accurately summarizes the main security fix, which adds validation to verify upload ownership before file finalization.
Linked Issues check	✅ Passed	The code changes fully implement all requirements from issue #38892: fetch the upload by _id, verify userId ownership, verify room association (rid), throw error-invalid-file on validation failure, and only call updateFileComplete after validation passes.
Out of Scope Changes check	✅ Passed	The changes are narrowly focused on adding authorization validation checks before upload finalization, with no modifications to unrelated functionality or scope creep beyond the stated security fix objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 1 file

[sahillllllllll-bit]

@KevLehman please review this if you have time :) and sorry for the mention

[KevLehman]

Closing in favor of #39010, thx for the work!