Skip to content

🪪 fix: Scope Message Conversation Access#13183

Merged
danny-avila merged 2 commits into
devfrom
danny-avila/fix-message-conversation-auth
May 18, 2026
Merged

🪪 fix: Scope Message Conversation Access#13183
danny-avila merged 2 commits into
devfrom
danny-avila/fix-message-conversation-auth

Conversation

@danny-avila

Copy link
Copy Markdown
Owner

I fixed a cross-user message injection path by making conversation ownership checks and message persistence use the same validated conversation ID.

  • Reject mismatched URL, body, and nested message conversationId values before the message route reaches database writes.
  • Force POST /api/messages/:conversationId to persist the validated URL conversationId instead of trusting the request body.
  • Scope message reads, history loading, thread gap sync, and conversation message aggregation by authenticated user.
  • Added regressions for middleware mismatch rejection, route-level save/read scoping, BaseClient history scoping, and saveConvo message aggregation.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • Ran npm ci --ignore-scripts --no-audit --no-fund to install dependencies for this worktree.
  • Ran npm run build:data-provider, npm --workspace packages/data-schemas run build, and npm --workspace packages/api run build so local workspace package entrypoints resolve during tests.
  • Ran npm --workspace api test -- server/middleware/__tests__/validateMessageReq.spec.js server/routes/__tests__/messages-delete.spec.js app/clients/specs/BaseClient.test.js --runInBand.
  • Ran npm --workspace packages/data-schemas run test:ci -- src/methods/conversation.spec.ts --runInBand.

Test Configuration:

  • Node.js: local workspace runtime
  • Database: mongodb-memory-server via targeted Jest suites

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

Copilot AI review requested due to automatic review settings May 18, 2026 20:57

Copy link
Copy Markdown
Owner Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a cross-user message injection/IDOR path by ensuring message persistence and retrieval are consistently scoped to the authenticated user and to a single, validated conversationId (preferring the URL param and rejecting mismatches).

Changes:

  • Enforce conversationId consistency across URL/body/nested payload in validateMessageReq and persist the validated URL conversationId on POST.
  • Scope message reads (routes, BaseClient history load, thread gap sync) by authenticated user.
  • Scope saveConvo message aggregation (getMessages) by user and add/adjust regression tests for the above behaviors.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/data-schemas/src/methods/conversation.ts Scope saveConvo message aggregation query by user.
packages/data-schemas/src/methods/conversation.spec.ts Update expectations to assert user-scoped getMessages query.
api/server/services/Threads/manage.js Scope thread gap DB message reads by authenticated user.
api/server/routes/messages.js Scope message reads by user; force POST persistence to use URL conversationId.
api/server/routes/tests/messages-delete.spec.js Add route-level regression tests for POST persistence and user-scoped reads.
api/server/middleware/validateMessageReq.js Reject mismatched conversationId values (URL/body/nested) before DB access.
api/server/middleware/tests/validateMessageReq.spec.js Add tests for mismatch rejection and ownership validation path.
api/app/clients/BaseClient.js Scope loadHistory DB reads by this.user.
api/app/clients/specs/BaseClient.test.js Add regression test asserting loadHistory uses user-scoped reads.
Comments suppressed due to low confidence (1)

api/server/routes/messages.js:317

  • db.getMessages() returns an array; when no record matches, it returns [] which is truthy, so if (!message) won’t trigger and the route responds 200 with an empty array. Consider selecting the first result (e.g., const message = (await db.getMessages(...))?.[0];) and checking for absence (if (!message)) so missing/unauthorized messages correctly return 404 and the response shape is consistent.
    const { conversationId, messageId } = req.params;
    const message = await db.getMessages(
      { conversationId, messageId, user: req.user.id },
      '-_id -__v -user',
    );
    if (!message) {
      return res.status(404).json({ error: 'Message not found' });
    }
    res.status(200).json(message);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

Copy link
Copy Markdown
Contributor

GitNexus: 🚀 deployed

The LibreChat-pr-13183 index is now live on the MCP server.
Deploy run

@danny-avila
danny-avila marked this pull request as ready for review May 18, 2026 21:33
@danny-avila
danny-avila changed the base branch from main to dev May 18, 2026 21:33
@danny-avila
danny-avila merged commit 5b66196 into dev May 18, 2026
12 checks passed
@danny-avila
danny-avila deleted the danny-avila/fix-message-conversation-auth branch May 18, 2026 21:34
patricia2510 pushed a commit to lexaeon-org/libre-chat that referenced this pull request May 21, 2026
* fix: Scope message conversation access

* style: Format message route query
fuuuzzy pushed a commit to fuuuzzy/LibreChat that referenced this pull request May 22, 2026
* fix: Scope message conversation access

* style: Format message route query
ThomasVuNguyen pushed a commit to ThomasVuNguyen/LibreChat that referenced this pull request Jul 15, 2026
* fix: Scope message conversation access

* style: Format message route query
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.

2 participants