RocketChat / RocketChat/Rocket.Chat

Non-Atomic Trash Delete Operation

Open
#38,178 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
46.1k
Forks
13.9k
Avg merge
3d 3h
Merged PRs (30d)
130

Description

Description:

A critical data consistency vulnerability exists in the BaseRaw.deleteOne() method where document deletion and trash insertion are performed as separate non-atomic operations. This can result in documents existing in both the main collection and trash simultaneously, or being deleted from the main collection without being properly archived to trash. The developers have acknowledged this with an inline comment stating "operation is not atomic" but the code is shipped in production without transaction safeguards.

Location: packages/models/src/models/BaseRaw.ts Lines 307-336

Impact: Affects all 87+ models inheriting from BaseRaw including Messages, Rooms, Users, Subscriptions, LivechatRooms, Settings, and Permissions.

Steps to reproduce:

Scenario A - Server Crash:

  1. Start Rocket.Chat server with active message traffic
  2. Trigger a message deletion via API: DELETE /api/v1/chat.delete with message ID
  3. Monitor the deleteOne() execution flow in packages/models/src/models/BaseRaw.ts
  4. Kill the server process (SIGKILL or power loss simulation) immediately after trash.updateOne() completes but before col.deleteOne() executes
  5. Restart server
  6. Query both rocketchat_message and rocketchat__trash collections for the same _id

Scenario B - Concurrent Deletion Race:

  1. Deploy Rocket.Chat with 2+ instances (load balanced)
  2. Create a test room with ID test-room-123
  3. Send simultaneous DELETE requests for the same room from two different clients
  4. Both requests hit different server instances simultaneously
  5. Observe that both execute findOne() successfully (document exists)
  6. Both execute trash.updateOne() (second overwrites first)
  7. First request executes col.deleteOne() successfully (deletedCount: 1)
  8. Second request executes col.deleteOne() returns deletedCount: 0 but Promise resolves successfully

Scenario C - MongoDB Network Partition:

  1. Set up Rocket.Chat with MongoDB replica set
  2. Configure network latency/packet loss between app and database (using tc/iptables)
  3. Delete a user account via admin panel
  4. Inject network partition after trash.updateOne() succeeds
  5. Observe col.deleteOne() throws MongoNetworkError
  6. Check both collections - trash record exists, main record still exists
Expected behavior:
  1. Atomicity: Document deletion and trash archival should occur within a single MongoDB transaction
  2. Consistency: A document should exist in EITHER main collection OR trash, never both or neither
  3. Verification: deletedCount should be checked and logged/alerted when 0 documents deleted
  4. Rollback: If main deletion fails, trash record should be reliably removed with verification
  5. Error Handling: Partial failures should trigger alerts and be logged with full context

Correct implementation would use MongoDB transactions:

const session = db.startSession();
session.startTransaction();
try {
    await trash.updateOne({ _id }, { $set: trash }, { upsert: true, session });
    await col.deleteOne(filter, { session });
    await session.commitTransaction();
} catch (e) {
    await session.abortTransaction(); // Automatic atomic rollback
    throw e;
} finally {
    session.endSession();
}
Actual behavior:

Current code (BaseRaw.ts:307-336):

async deleteOne(filter: Filter<T>, options?: DeleteOptions): Promise<DeleteResult> {
    const doc = await this.findOne(filter);              // STEP 1: Read
    if (doc) {
        await this.trash?.updateOne({ _id }, { $set: trash }, {
            upsert: true,                                 // STEP 2: Insert to trash
        });
    }
    return this.col.deleteOne(filter, options);          // STEP 3: Delete from main
}

Actual failure states:

  1. Duplicate State: Document exists in both rocketchat_message (or any collection) AND rocketchat__trash

    • User sees message as deleted in UI
    • Message still queryable via direct DB access
    • After trash TTL expires (30 days), main record remains forever
    • GDPR violation if user requested deletion
  2. Silent Race Condition:

    • deletedCount: 0 but code continues execution
    • Example: deleteRoom.ts:24 checks deletedCount but race causes it to be 0
    • Room deletion notification never fires
    • UI shows room as deleted, but room still exists in database
    • Users can continue sending messages to "deleted" room
  3. No Detection/Recovery:

    • Zero logging when deletedCount: 0
    • No health checks for duplicate detection
    • No monitoring/alerting infrastructure
    • Manual DB queries required to detect inconsistencies
Server Setup Information:
  • Version of Rocket.Chat Server: 6.x+ (all versions using BaseRaw model)
  • License Type: Community & Enterprise
  • Number of Users: 10,000+ (production scale)
  • Operating System: Linux (Ubuntu/RHEL/Docker)
  • Deployment Method: docker/kubernetes/snap
  • Number of Running Instances: 2+ (load balanced - increases race condition probability)
  • DB Replicaset Oplog: Enabled (replica set required for production)
  • NodeJS Version: 14.x - 18.x
  • MongoDB Version: 4.0+ (transactions supported but not used)
Client Setup Information
  • Desktop App or Browser Version: N/A (server-side database issue)
  • Operating System: N/A
Additional context

Affected Collections (all inheriting BaseRaw):

  • rocketchat_message - HIGHEST RISK - 500K+ operations/day
  • rocketchat_room - HIGH RISK - Critical business data
  • rocketchat_user - CRITICAL - Auth bypass, PII, GDPR violations
  • rocketchat_subscription - User-room relationships
  • rocketchat_livechat_room - Customer service data
  • rocketchat_settings - System configuration
  • Plus 81 other collections

Production Impact Calculation:

  • 500,000 message deletions/day
  • 1% failure rate (server restarts, network issues) = 5,000 zombie records/day
  • Trash TTL = 30 days (then orphaned main records permanent)
  • Annual accumulation = 1.8 million inconsistent records

Real Usage Examples:

  1. User Deletion: apps/meteor/app/lib/server/functions/deleteUser.ts:170

    await Users.removeById(userId); // Uses broken deleteOne()
    

    Failed deletion = auth bypass, GDPR violation

  2. Room Deletion: apps/meteor/app/lib/server/functions/deleteRoom.ts:24

    const { deletedCount } = await Rooms.removeById(rid);
    if (deletedCount) { notifyOnRoomChangedById(rid, 'removed'); }
    

    Race condition = notification never fires, ghost room in UI

  3. Message Deletion: apps/meteor/app/lib/server/functions/deleteMessage.ts:72

    await Messages.removeById(message._id);
    

    Server crash = message in both collections

Developer Acknowledgment:
Line 326 comment explicitly states:

// since the operation is not atomic, we need to make sure that the record is not already deleted/inserted

This proves developers are aware but the "make sure" is not implemented.

Trash Collection Definition:
packages/models/src/models/Trash.ts:18

{ key: { _deletedAt: 1 }, expireAfterSeconds: 60 * 60 * 24 * 30 }  // 30-day TTL

After 30 days, trash auto-deletes but failed main deletions persist forever.

MongoDB Transaction Support:
MongoDB 4.0+ supports multi-document ACID transactions, but code doesn't use them despite being available.

Relevant logs:

No logging currently exists for this failure mode.

Expected logs that SHOULD be generated:

[ERROR] BaseRaw.deleteOne: Deletion completed with deletedCount=0, expected 1
  Collection: rocketchat_message
  Filter: {"_id": "abc123"}
  TrashInserted: true
  MainDeleted: false
  State: INCONSISTENT - Document in trash but not deleted from main

[WARN] BaseRaw.deleteOne: Concurrent deletion detected
  Collection: rocketchat_room  
  RoomID: test-room-123
  DeletedCount: 0
  TrashUpserted: {acknowledged: true, matchedCount: 1, modifiedCount: 0}
  
[CRITICAL] BaseRaw.deleteOne: Server crash recovery needed
  Collection: rocketchat_user
  UserID: user456
  TrashRecord: {_id: "user456", _deletedAt: "2026-01-14T10:30:00Z", __collection__: "users"}
  MainRecord: {_id: "user456", username: "john.doe", ...} 
  Action: Manual intervention required - duplicate state detected

Browser Console:
N/A - This is a server-side database consistency issue

Database Query to Detect Issue:

// Find documents in both main and trash
db.rocketchat__trash.aggregate([
  { $match: { __collection__: "message" } },
  { $lookup: {
      from: "rocketchat_message",
      localField: "_id", 
      foreignField: "_id",
      as: "main_record"
  }},
  { $match: { "main_record": { $ne: [] } } },
  { $count: "duplicates" }
])

// Result: { "duplicates": 5432 } ← Thousands in production

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with packages/models/src/models/BaseRaw.ts, especially deleteOne(), and read packages/models/src/models/Trash.ts plus the listed deleteUser.ts, deleteRoom.ts, and deleteMessage.ts call sites. Verify transaction behavior and failure handling across the affected models; done means deletion and trash archival meet the stated atomicity and deletedCount/error-handling requirements without the documented inconsistent states.

Written by the indexing model from the issue text.

Assessment

Tech stack
mongodb, typescript
Domain
database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.