mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🛡️ fix: Escape People Picker Search Regex (#13169)
This commit is contained in:
parent
d2958bcfea
commit
89d10a0b83
6 changed files with 165 additions and 26 deletions
|
|
@ -385,15 +385,17 @@ const getUserEffectivePermissions = async (req, res) => {
|
|||
*/
|
||||
const searchPrincipals = async (req, res) => {
|
||||
try {
|
||||
const { q: query, limit = 20, types } = req.query;
|
||||
const { q: rawQuery, limit = 20, types } = req.query;
|
||||
|
||||
if (!query || query.trim().length === 0) {
|
||||
if (typeof rawQuery !== 'string' || rawQuery.trim().length === 0) {
|
||||
return res.status(400).json({
|
||||
error: 'Query parameter "q" is required and must not be empty',
|
||||
});
|
||||
}
|
||||
|
||||
if (query.trim().length < 2) {
|
||||
const query = rawQuery.trim();
|
||||
|
||||
if (query.length < 2) {
|
||||
return res.status(400).json({
|
||||
error: 'Query must be at least 2 characters long',
|
||||
});
|
||||
|
|
@ -410,7 +412,7 @@ const searchPrincipals = async (req, res) => {
|
|||
typeFilters = validTypes.length > 0 ? validTypes : null;
|
||||
}
|
||||
|
||||
const localResults = await db.searchPrincipals(query.trim(), searchLimit, typeFilters);
|
||||
const localResults = await db.searchPrincipals(query, searchLimit, typeFilters);
|
||||
let allPrincipals = [...localResults];
|
||||
|
||||
const useEntraId = entraIdPrincipalFeatureEnabled(req.user);
|
||||
|
|
@ -437,7 +439,7 @@ const searchPrincipals = async (req, res) => {
|
|||
const graphResults = await searchEntraIdPrincipals(
|
||||
accessToken,
|
||||
req.user.openidId,
|
||||
query.trim(),
|
||||
query,
|
||||
graphType,
|
||||
searchLimit - localResults.length,
|
||||
);
|
||||
|
|
@ -466,7 +468,7 @@ const searchPrincipals = async (req, res) => {
|
|||
}
|
||||
const scoredResults = allPrincipals.map((item) => ({
|
||||
...item,
|
||||
_searchScore: db.calculateRelevanceScore(item, query.trim()),
|
||||
_searchScore: db.calculateRelevanceScore(item, query),
|
||||
}));
|
||||
|
||||
const finalResults = db
|
||||
|
|
@ -478,7 +480,7 @@ const searchPrincipals = async (req, res) => {
|
|||
});
|
||||
|
||||
res.status(200).json({
|
||||
query: query.trim(),
|
||||
query,
|
||||
limit: searchLimit,
|
||||
types: typeFilters,
|
||||
results: finalResults,
|
||||
|
|
@ -492,7 +494,6 @@ const searchPrincipals = async (req, res) => {
|
|||
logger.error('Error searching principals:', error);
|
||||
res.status(500).json({
|
||||
error: 'Failed to search principals',
|
||||
details: error.message,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -43,7 +43,8 @@ jest.mock('~/server/services/GraphApiService', () => ({
|
|||
searchEntraIdPrincipals: jest.fn(),
|
||||
}));
|
||||
|
||||
const { updateResourcePermissions } = require('../PermissionsController');
|
||||
const db = require('~/models');
|
||||
const { updateResourcePermissions, searchPrincipals } = require('../PermissionsController');
|
||||
|
||||
const createMockReq = (overrides = {}) => ({
|
||||
params: { resourceType: ResourceType.AGENT, resourceId: '507f1f77bcf86cd799439011' },
|
||||
|
|
@ -67,6 +68,77 @@ describe('PermissionsController', () => {
|
|||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('searchPrincipals', () => {
|
||||
beforeEach(() => {
|
||||
db.searchPrincipals.mockResolvedValue([]);
|
||||
db.calculateRelevanceScore.mockReturnValue(50);
|
||||
db.sortPrincipalsByRelevance.mockImplementation((results) => results);
|
||||
});
|
||||
|
||||
it('rejects non-string query parameters', async () => {
|
||||
const req = createMockReq({
|
||||
query: { q: ['alice'] },
|
||||
});
|
||||
const res = createMockRes();
|
||||
|
||||
await searchPrincipals(req, res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
error: 'Query parameter "q" is required and must not be empty',
|
||||
});
|
||||
expect(db.searchPrincipals).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('searches with the trimmed literal query', async () => {
|
||||
db.searchPrincipals.mockResolvedValue([
|
||||
{
|
||||
id: 'user-1',
|
||||
type: PrincipalType.USER,
|
||||
name: 'Regex [invalid User',
|
||||
source: 'local',
|
||||
},
|
||||
]);
|
||||
|
||||
const req = createMockReq({
|
||||
query: { q: ' [invalid ', limit: '5', types: PrincipalType.USER },
|
||||
});
|
||||
const res = createMockRes();
|
||||
|
||||
await searchPrincipals(req, res);
|
||||
|
||||
expect(db.searchPrincipals).toHaveBeenCalledWith('[invalid', 5, [PrincipalType.USER]);
|
||||
expect(db.calculateRelevanceScore).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ name: 'Regex [invalid User' }),
|
||||
'[invalid',
|
||||
);
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
query: '[invalid',
|
||||
limit: 5,
|
||||
count: 1,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not expose internal error details on search failures', async () => {
|
||||
db.searchPrincipals.mockRejectedValue(new Error('database failure with internal detail'));
|
||||
|
||||
const req = createMockReq({
|
||||
query: { q: 'alice' },
|
||||
});
|
||||
const res = createMockRes();
|
||||
|
||||
await searchPrincipals(req, res);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(500);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
error: 'Failed to search principals',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateResourcePermissions — favorites cleanup', () => {
|
||||
const agentObjectId = new mongoose.Types.ObjectId().toString();
|
||||
const revokedUserId = new mongoose.Types.ObjectId().toString();
|
||||
|
|
|
|||
|
|
@ -476,6 +476,34 @@ describe('User Methods - Database Tests', () => {
|
|||
expect(results).toEqual([]);
|
||||
});
|
||||
|
||||
test('should treat regex metacharacters as literal search text', async () => {
|
||||
await User.create({
|
||||
name: 'Literal .* User',
|
||||
email: 'literal-star@test.com',
|
||||
username: 'literal-star',
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
const results = await methods.searchUsers({ searchPattern: '.*' });
|
||||
|
||||
expect(results).toHaveLength(1);
|
||||
expect((results[0] as unknown as t.IUser).name).toBe('Literal .* User');
|
||||
});
|
||||
|
||||
test('should handle invalid regex syntax as literal search text', async () => {
|
||||
await User.create({
|
||||
name: 'Regex [invalid User',
|
||||
email: 'regex-invalid@test.com',
|
||||
username: 'regex-invalid',
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
const results = await methods.searchUsers({ searchPattern: '[invalid' });
|
||||
|
||||
expect(results).toHaveLength(1);
|
||||
expect((results[0] as unknown as t.IUser).name).toBe('Regex [invalid User');
|
||||
});
|
||||
|
||||
test('should apply field selection', async () => {
|
||||
const results = await methods.searchUsers({
|
||||
searchPattern: 'john',
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import mongoose, { FilterQuery } from 'mongoose';
|
||||
import type { RefillIntervalUnit } from 'librechat-data-provider';
|
||||
import type { IUser, BalanceConfig, CreateUserRequest, UserDeleteResult } from '~/types';
|
||||
import { escapeRegExp } from '~/utils/string';
|
||||
import { signPayload } from '~/crypto';
|
||||
|
||||
/** Default JWT session expiry: 15 minutes in milliseconds */
|
||||
|
|
@ -259,7 +260,8 @@ export function createUserMethods(mongoose: typeof import('mongoose')) {
|
|||
return [];
|
||||
}
|
||||
|
||||
const regex = new RegExp(searchPattern.trim(), 'i');
|
||||
const trimmedPattern = searchPattern.trim();
|
||||
const regex = new RegExp(escapeRegExp(trimmedPattern), 'i');
|
||||
const User = mongoose.models.User;
|
||||
|
||||
const query = User.find({
|
||||
|
|
@ -273,8 +275,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')) {
|
|||
const users = await query.lean<IUser[]>();
|
||||
|
||||
// Score results by relevance
|
||||
const exactRegex = new RegExp(`^${searchPattern.trim()}$`, 'i');
|
||||
const startsWithPattern = searchPattern.trim().toLowerCase();
|
||||
const startsWithPattern = trimmedPattern.toLowerCase();
|
||||
|
||||
const scoredUsers = users.map((user) => {
|
||||
const searchableFields = [user.name, user.email, user.username].filter(
|
||||
|
|
@ -287,7 +288,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')) {
|
|||
let score = 0;
|
||||
|
||||
// Exact match gets highest score
|
||||
if (exactRegex.test(field)) {
|
||||
if (fieldLower === startsWithPattern) {
|
||||
score = 100;
|
||||
}
|
||||
// Starts with query gets high score
|
||||
|
|
@ -298,7 +299,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')) {
|
|||
else if (fieldLower.includes(startsWithPattern)) {
|
||||
score = 50;
|
||||
}
|
||||
// Default score for regex match
|
||||
// Default score for database match
|
||||
else {
|
||||
score = 10;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ describe('userGroup methods', () => {
|
|||
{ name: 'Engineering', source: 'local', description: 'Eng team' },
|
||||
{ name: 'Design', source: 'local', email: 'design@co.com' },
|
||||
{ name: 'Entra Eng', source: 'entra', idOnTheSource: 'ext-1' },
|
||||
{ name: 'Literal .* Group', source: 'local' },
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -120,6 +121,12 @@ describe('userGroup methods', () => {
|
|||
expect(results[0].name).toBe('Engineering');
|
||||
});
|
||||
|
||||
it('treats regex metacharacters as literal text', async () => {
|
||||
const results = await methods.findGroupsByNamePattern('.*');
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0].name).toBe('Literal .* Group');
|
||||
});
|
||||
|
||||
it('filters by source when provided', async () => {
|
||||
const results = await methods.findGroupsByNamePattern('eng', 'entra');
|
||||
expect(results).toHaveLength(1);
|
||||
|
|
@ -525,7 +532,7 @@ describe('userGroup methods', () => {
|
|||
expect(score).toBe(50);
|
||||
});
|
||||
|
||||
it('returns 10 (default) when no substring or exact match — regex fallback', () => {
|
||||
it('returns 10 (default) when no substring or exact match', () => {
|
||||
const score = methods.calculateRelevanceScore(
|
||||
{ type: PrincipalType.USER, name: 'bob', source: 'local' },
|
||||
'zzz',
|
||||
|
|
@ -573,12 +580,12 @@ describe('userGroup methods', () => {
|
|||
expect(score).toBe(80);
|
||||
});
|
||||
|
||||
it('returns 100 when regex pattern matches exactly via dot wildcard', () => {
|
||||
it('does not treat regex metacharacters as wildcards', () => {
|
||||
const score = methods.calculateRelevanceScore(
|
||||
{ type: PrincipalType.USER, name: 'xYz', source: 'local' },
|
||||
'x.z',
|
||||
);
|
||||
expect(score).toBe(100);
|
||||
expect(score).toBe(10);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -671,6 +678,36 @@ describe('userGroup methods', () => {
|
|||
expect(results).toEqual([]);
|
||||
});
|
||||
|
||||
it('treats regex metacharacters as literal search text', async () => {
|
||||
await User.create({
|
||||
name: 'Literal .* User',
|
||||
email: 'literal-star@test.com',
|
||||
username: 'literal-star',
|
||||
password: 'password123',
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
const results = await methods.searchPrincipals('.*', 10, [PrincipalType.USER]);
|
||||
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0].name).toBe('Literal .* User');
|
||||
});
|
||||
|
||||
it('handles invalid regex syntax as literal search text', async () => {
|
||||
await User.create({
|
||||
name: 'Regex [invalid User',
|
||||
email: 'regex-invalid@test.com',
|
||||
username: 'regex-invalid',
|
||||
password: 'password123',
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
const results = await methods.searchPrincipals('[invalid', 10, [PrincipalType.USER]);
|
||||
|
||||
expect(results).toHaveLength(1);
|
||||
expect(results[0].name).toBe('Regex [invalid User');
|
||||
});
|
||||
|
||||
it('finds matching users', async () => {
|
||||
const results = await methods.searchPrincipals('alice');
|
||||
const userResults = results.filter((r) => r.type === PrincipalType.USER);
|
||||
|
|
|
|||
|
|
@ -86,7 +86,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
session?: ClientSession,
|
||||
): Promise<IGroup[]> {
|
||||
const Group = mongoose.models.Group as Model<IGroup>;
|
||||
const regex = new RegExp(namePattern, 'i');
|
||||
const regex = new RegExp(escapeRegExp(namePattern), 'i');
|
||||
const query: Record<string, unknown> = {
|
||||
$or: [{ name: regex }, { email: regex }, { description: regex }],
|
||||
};
|
||||
|
|
@ -436,8 +436,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
* @returns Relevance score (0-100)
|
||||
*/
|
||||
function calculateRelevanceScore(item: TPrincipalSearchResult, searchPattern: string): number {
|
||||
const exactRegex = new RegExp(`^${searchPattern}$`, 'i');
|
||||
const startsWithPattern = searchPattern.toLowerCase();
|
||||
const normalizedPattern = searchPattern.toLowerCase();
|
||||
|
||||
/** Get searchable text based on type */
|
||||
const searchableFields =
|
||||
|
|
@ -453,16 +452,16 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
let score = 0;
|
||||
|
||||
/** Exact match gets highest score */
|
||||
if (exactRegex.test(field)) {
|
||||
if (fieldLower === normalizedPattern) {
|
||||
score = 100;
|
||||
} else if (fieldLower.startsWith(startsWithPattern)) {
|
||||
} else if (fieldLower.startsWith(normalizedPattern)) {
|
||||
/** Starts with query gets high score */
|
||||
score = 80;
|
||||
} else if (fieldLower.includes(startsWithPattern)) {
|
||||
} else if (fieldLower.includes(normalizedPattern)) {
|
||||
/** Contains query gets medium score */
|
||||
score = 50;
|
||||
} else {
|
||||
/** Default score for regex match */
|
||||
/** Default score for database match */
|
||||
score = 10;
|
||||
}
|
||||
|
||||
|
|
@ -551,6 +550,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
}
|
||||
|
||||
const trimmedPattern = searchPattern.trim();
|
||||
const escapedPattern = escapeRegExp(trimmedPattern);
|
||||
const promises: Promise<TPrincipalSearchResult[]>[] = [];
|
||||
|
||||
if (!typeFilter || typeFilter.includes(PrincipalType.USER)) {
|
||||
|
|
@ -558,7 +558,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
const userFields = 'name email username avatar provider idOnTheSource';
|
||||
/** For now, we'll use a direct query instead of searchUsers */
|
||||
const User = mongoose.models.User as Model<IUser>;
|
||||
const regex = new RegExp(trimmedPattern, 'i');
|
||||
const regex = new RegExp(escapedPattern, 'i');
|
||||
const userQuery = User.find({
|
||||
$or: [{ name: regex }, { email: regex }, { username: regex }],
|
||||
})
|
||||
|
|
@ -601,7 +601,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
|
|||
if (!typeFilter || typeFilter.includes(PrincipalType.ROLE)) {
|
||||
const Role = mongoose.models.Role as Model<IRole>;
|
||||
if (Role) {
|
||||
const regex = new RegExp(trimmedPattern, 'i');
|
||||
const regex = new RegExp(escapedPattern, 'i');
|
||||
const roleQuery = Role.find({ name: regex }).select('name').limit(limitPerType);
|
||||
|
||||
if (session) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue