🔣 refactor: Replace Mongo Filters With Typed Criteria (#15580)

* ♻️ refactor: Take Typed Criteria, Not Mongo Filters, for Actions and Assistants

`createActionMethods` and `createAssistantMethods` accepted `FilterQuery<T>`,
putting Mongoose's query language in the package's public surface: every caller
wrote raw Mongo, and no engine other than Mongo could satisfy the signatures.

Both now take domain criteria — `ActionQuery` and `AssistantQuery` — naming
concepts rather than stored fields, with an array meaning "any of these". A
per-domain field map translates them, so `FilterQuery` survives only inside the
two translators.

`buildFilter` throws on a criterion the field map does not cover. Callers in
`api/` are JavaScript and get no compile-time checking, and the server sets
`strictQuery`, under which a silently dropped criterion widens the filter
instead of narrowing it — an unscoped `findOne` returns an arbitrary document
rather than none. Failing closed is the only safe default.

`AssistantQuery.avatarFilepath` covers the avatar-authorization lookup, which
queries a nested path no plain field name reaches.

No behaviour change: every migrated call site produces the same filter as before.

* 🧪 test: Update Avatar Authorization Assertion to Typed Criteria

`validateImages.spec.js` asserted the old `{ 'avatar.filepath': { $in: [...] } }`
call shape for `getAssistant`. The earlier sweep matched on `action_id`,
`agent_id` and `assistant_id`, none of which appear in a dotted avatar path, so
this one assertion was missed.

The `getAgent` assertions in the same file keep the filter shape — that method
still takes a `FilterQuery`.

* 🔒 fix: Reject Unknown Criteria Before Omitting Undefined Ones

`buildFilter` skipped a criterion whose value was `undefined` before checking it
against the field map, so a JavaScript caller misspelling a key that happened to
carry `undefined` — `deleteActions({ agent_id: maybeId })` — produced `{}` and
reached `deleteMany` unscoped. That is the exact failure the guard exists to
prevent, defeated by the order of the two checks.

Validate every key first, then apply the undefined-omission rule. The lookup is
an own-property check so inherited names cannot resolve through the field map's
prototype: `{ toString: 'x' }` previously found `Function.prototype.toString`,
passed the truthiness test, and wrote a garbage filter key instead of throwing.

A recognized criterion left `undefined` is still omitted, so optional parameters
keep working.

* 🔒 fix: Reject Query Fragments as Criteria and Keep the Translator Internal

Three review findings on the criteria translator, all of the same shape: the
guard was narrower than the surface it protects.

Criterion values are now validated as scalars or lists of scalars. `matchAny`
copied any non-array through unchanged, so a JavaScript caller could pass
`{ agentId: { $ne: null } }` and have the operator land verbatim in the filter
— the fail-open case the field map exists to close. `null` is rejected too; it
would have matched documents missing the field entirely.

`matchAny`, `buildFilter` and `FieldMap` are no longer re-exported from the
package barrel. They emit Mongo, and shipping them as public API undercut the
point of the change; they were reachable as `require('@librechat/data-schemas')
.buildFilter`. `OneOrMany` moves to `types/query` because the domain query types
reference it and it names no engine — which also drops the `types` -> `utils`
import.

`loadActionSets` names `ActionQuery` through `import()` so the JSDoc type
actually resolves.
This commit is contained in:
Danny Avila 2026-09-04 11:54:21 -04:00 • committed by GitHub
parent a21c7944ee
commit 770238f22b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
29 changed files with 486 additions and 110 deletions

View file

@ -1530,7 +1530,7 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => {
const res = await post(approveBody());
expect(res.status).toBe(400);
expect(mockGetActions).toHaveBeenCalledWith({ agent_id: { $in: [AGENT_ID] } }, false);
expect(mockGetActions).toHaveBeenCalledWith({ agentId: [AGENT_ID] }, false);
expect(mockDecryptMetadata).not.toHaveBeenCalled();
expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled();
expect(mockInitializeClient).not.toHaveBeenCalled();

View file

@ -68,7 +68,7 @@ describe('duplicateAgent', () => {
await duplicateAgent(req, res);
expect(getAgent).toHaveBeenCalledWith({ id: 'agent_123' });
expect(getActions).toHaveBeenCalledWith({ agent_id: 'agent_123' }, true);
expect(getActions).toHaveBeenCalledWith({ agentId: 'agent_123' }, true);
expect(createAgent).toHaveBeenCalledWith(
expect.objectContaining({
id: 'agent_new_123',

View file

@ -1465,7 +1465,7 @@ const duplicateAgentHandler = async (req, res) => {
return res.status(subagentReferenceError.status).json(subagentReferenceError.body);
}
const originalActions = (await db.getActions({ agent_id: id }, true)) ?? [];
const originalActions = (await db.getActions({ agentId: id }, true)) ?? [];
const sanitizedActions = originalActions.map((action) => {
const metadata = { ...(action.metadata || {}) };
for (const field of sensitiveFields) {
@ -1550,7 +1550,7 @@ const duplicateAgentHandler = async (req, res) => {
const fullActionId = `${domain}${actionDelimiter}${newActionId}`;
const newAction = await db.updateAction(
{ action_id: newActionId, agent_id: newAgentId },
{ actionId: newActionId, agentId: newAgentId },
{
metadata: action.metadata,
agent_id: newAgentId,
@ -1575,7 +1575,7 @@ const duplicateAgentHandler = async (req, res) => {
try {
newAgent = await db.createAgent(newAgentData);
} catch (error) {
await db.deleteActions({ agent_id: newAgentId, user: userId }).catch((cleanupError) => {
await db.deleteActions({ agentId: newAgentId, user: userId }).catch((cleanupError) => {
logger.error(
'[/agents/:id/duplicate] Failed to clean up cloned Actions after Agent creation failed:',
cleanupError,
@ -2065,7 +2065,7 @@ const revertAgentVersionHandler = async (req, res) => {
.filter(Boolean);
const actions =
actionIds.length > 0
? ((await db.getActions({ agent_id: id, action_id: { $in: actionIds } }, true)) ?? [])
? ((await db.getActions({ agentId: id, actionId: actionIds }, true)) ?? [])
: [];
if (

View file

@ -104,7 +104,7 @@ const createAssistant = async (req, res) => {
createData.append_current_datetime = append_current_datetime;
}
const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData);
const document = await updateAssistantDoc({ assistantId: assistant.id }, createData);
if (azureModelIdentifier) {
assistant.model = azureModelIdentifier;
@ -220,14 +220,14 @@ const patchAssistant = async (req, res) => {
if (conversation_starters !== undefined) {
const conversationStartersUpdate = await updateAssistantDoc(
{ assistant_id },
{ assistantId: assistant_id },
{ conversation_starters },
);
updatedAssistant.conversation_starters = conversationStartersUpdate.conversation_starters;
}
if (append_current_datetime !== undefined) {
await updateAssistantDoc({ assistant_id }, { append_current_datetime });
await updateAssistantDoc({ assistantId: assistant_id }, { append_current_datetime });
updatedAssistant.append_current_datetime = append_current_datetime;
}
@ -418,7 +418,7 @@ const uploadAssistantAvatar = async (req, res) => {
const promises = [];
promises.push(
updateAssistantDoc(
{ assistant_id },
{ assistantId: assistant_id },
{
avatar: {
filepath: image.filepath,

View file

@ -94,7 +94,7 @@ const createAssistant = async (req, res) => {
createData.append_current_datetime = append_current_datetime;
}
const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData);
const document = await updateAssistantDoc({ assistantId: assistant.id }, createData);
if (azureModelIdentifier) {
assistant.model = azureModelIdentifier;
@ -135,7 +135,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => {
if (updateData?.conversation_starters) {
const conversationStartersUpdate = await updateAssistantDoc(
{ assistant_id: assistant_id },
{ assistantId: assistant_id },
{ conversation_starters: updateData.conversation_starters },
);
conversation_starters = conversationStartersUpdate.conversation_starters;
@ -145,7 +145,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => {
if (updateData?.append_current_datetime !== undefined) {
await updateAssistantDoc(
{ assistant_id: assistant_id },
{ assistantId: assistant_id },
{ append_current_datetime: updateData.append_current_datetime },
);
delete updateData.append_current_datetime;

View file

@ -40,7 +40,7 @@ const validateAuthor = async ({ req, openai, overrideEndpoint, overrideAssistant
return;
}
const assistantDoc = await getAssistant({ assistant_id, user: req.user.id });
const assistantDoc = await getAssistant({ assistantId: assistant_id, user: req.user.id });
if (assistantDoc) {
return;
}

View file

@ -311,13 +311,11 @@ describe('validateImageRequest middleware', () => {
expect(next).toHaveBeenCalled();
expect(getAssistant).toHaveBeenCalledWith(
{
'avatar.filepath': {
$in: [
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png',
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false',
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true',
],
},
avatarFilepath: [
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png',
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false',
'/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true',
],
},
{ _id: 1, assistant_id: 1, endpoint: 1 },
);

View file

@ -68,9 +68,7 @@ router.get('/', async (req, res) => {
const editableAgentIds = agentsResponse.data.map((agent) => agent.id);
const actions =
editableAgentIds.length > 0
? await db.getActions({ agent_id: { $in: editableAgentIds } })
: [];
editableAgentIds.length > 0 ? await db.getActions({ agentId: editableAgentIds }) : [];
for (const action of actions) {
if (blockFilteredActionProjection(req.config?.filters, res, action)) {
@ -171,7 +169,7 @@ router.post(
// Permissions already validated by middleware - load agent directly
initialPromises.push(db.getAgent({ id: agent_id }));
if (requestedActionId) {
initialPromises.push(db.getActions({ action_id: requestedActionId }, true));
initialPromises.push(db.getActions({ actionId: requestedActionId }, true));
}
/** @type {[Agent, [Action|undefined]]} */
@ -254,7 +252,7 @@ router.post(
/** @type {Action} */
const updatedAction = await db.updateAction(
{ action_id: requestedActionId ?? action_id, agent_id },
{ actionId: requestedActionId ?? action_id, agentId: agent_id },
actionUpdateData,
);
@ -323,7 +321,7 @@ router.delete(
{ tools: updatedTools, actions: updatedActions },
{ updatingUserId: req.user.id, forceVersion: true },
);
const deleted = await db.deleteAction({ action_id, agent_id });
const deleted = await db.deleteAction({ actionId: action_id, agentId: agent_id });
if (!deleted) {
logger.warn('[Agent Action Delete] No matching action document found', {
action_id,

View file

@ -70,9 +70,9 @@ router.post('/:assistant_id', async (req, res) => {
const { openai } = await getOpenAIClient({ req, res });
initialPromises.push(db.getAssistant({ assistant_id }));
initialPromises.push(db.getAssistant({ assistantId: assistant_id }));
initialPromises.push(openai.beta.assistants.retrieve(assistant_id));
!!_action_id && initialPromises.push(db.getActions({ action_id }, true));
!!_action_id && initialPromises.push(db.getActions({ actionId: action_id }, true));
/** @type {[AssistantDocument, Assistant, [Action|undefined]]} */
const [assistant_data, assistant, actions_result] = await Promise.all(initialPromises);
@ -150,7 +150,7 @@ router.post('/:assistant_id', async (req, res) => {
if (!assistant_data) {
assistantUpdateData.user = req.user.id;
}
promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData));
promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData));
// Only update user field for new actions
const actionUpdateData = { metadata, assistant_id };
@ -158,7 +158,9 @@ router.post('/:assistant_id', async (req, res) => {
// For new actions, use the assistant owner's user ID
actionUpdateData.user = assistant_user || req.user.id;
}
promises.push(db.updateAction({ action_id, assistant_id }, actionUpdateData));
promises.push(
db.updateAction({ actionId: action_id, assistantId: assistant_id }, actionUpdateData),
);
/** @type {[AssistantDocument, Action]} */
let [assistantDocument, updatedAction] = await Promise.all(promises);
@ -200,7 +202,7 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => {
const { openai } = await getOpenAIClient({ req, res });
const initialPromises = [];
initialPromises.push(db.getAssistant({ assistant_id }));
initialPromises.push(db.getAssistant({ assistantId: assistant_id }));
initialPromises.push(openai.beta.assistants.retrieve(assistant_id));
/** @type {[AssistantDocument, Assistant]} */
@ -238,8 +240,8 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => {
if (!assistant_data) {
assistantUpdateData.user = req.user.id;
}
promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData));
promises.push(db.deleteAction({ action_id, assistant_id }));
promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData));
promises.push(db.deleteAction({ actionId: action_id, assistantId: assistant_id }));
const [, deletedAction] = await Promise.all(promises);
if (!deletedAction) {

View file

@ -289,7 +289,7 @@ router.delete('/', async (req, res) => {
/* Handle assistant unlinking even if no valid files to delete */
if (req.body.assistant_id && req.body.tool_resource && dbFiles.length === 0) {
const assistant = await db.getAssistant({
assistant_id: req.body.assistant_id,
assistantId: req.body.assistant_id,
});
const toolResourceFiles = assistant?.tool_resources?.[req.body.tool_resource]?.file_ids ?? [];

View file

@ -57,7 +57,7 @@ const validateAndUpdateTool = async ({ req, tool, assistant_id }) => {
}
if (!toolNameRegex.test(tool.function.name)) {
const [functionName, domain] = tool.function.name.split(actionDelimiter);
actions = await getActions({ assistant_id, user: req.user.id }, true);
actions = await getActions({ assistantId: assistant_id, user: req.user.id }, true);
const matchingActions = actions.filter((action) => {
const metadata = action.metadata;
if (!metadata) {
@ -157,14 +157,11 @@ async function domainParser(domain, inverse = false) {
/**
* Loads action sets based on the user and assistant ID.
*
* @param {Object} searchParams - The parameters for loading action sets.
* @param {string} searchParams.user - The user identifier.
* @param {string} [searchParams.agent_id]- The agent identifier.
* @param {string} [searchParams.assistant_id]- The assistant identifier.
* @param {import('@librechat/data-schemas').ActionQuery} query - The criteria for loading action sets.
* @returns {Promise<Action[] | null>} A promise that resolves to an array of actions or `null` if no match.
*/
async function loadActionSets(searchParams) {
return await getActions(searchParams, true);
async function loadActionSets(query) {
return await getActions(query, true);
}
/**
@ -513,8 +510,8 @@ async function decryptMetadata(metadata) {
*/
const deleteAssistantActions = async ({ req, assistant_id }) => {
try {
await deleteActions({ assistant_id, user: req.user.id });
await deleteAssistant({ assistant_id, user: req.user.id });
await deleteActions({ assistantId: assistant_id, user: req.user.id });
await deleteAssistant({ assistantId: assistant_id, user: req.user.id });
} catch (error) {
const message = 'Trouble deleting Assistant Actions for Assistant ID: ' + assistant_id;
logger.error(message, error);

View file

@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => {
});
if (assistant_id) {
const assistantDoc = await getAssistant({ assistant_id });
const assistantDoc = await getAssistant({ assistantId: assistant_id });
if (assistantDoc) {
// Create a clean assistant object with only the needed properties

View file

@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => {
});
if (assistant_id) {
const assistantDoc = await getAssistant({ assistant_id });
const assistantDoc = await getAssistant({ assistantId: assistant_id });
if (assistantDoc) {
endpointOption.assistant = {
append_current_datetime: assistantDoc.append_current_datetime,

View file

@ -199,7 +199,7 @@ const prepareActionSnapshotForTools = async ({ agentId, toolNames, filters, decr
if (!toolNames.some((toolName) => isActionTool(toolName))) {
return null;
}
const storedActions = (await loadActionSets({ agent_id: agentId })) ?? [];
const storedActions = (await loadActionSets({ agentId })) ?? [];
if (storedActions.length === 0) {
return { storedActions, actionSets: [] };
}
@ -518,7 +518,7 @@ async function processRequiredActions(client, requiredActions) {
/** @type {Action[]} */
const storedActions =
(await loadActionSets({
assistant_id: client.req.body.assistant_id,
assistantId: client.req.body.assistant_id,
})) ?? [];
const actionSets = await prepareStoredActionsForUse({
actions: storedActions,

View file

@ -2330,7 +2330,7 @@ describe('ToolService - Action Capability Gating', () => {
definitionsOnly: false,
});
expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' });
expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' });
});
it('blocks persisted action metadata before generic tool initialization', async () => {
@ -3013,7 +3013,7 @@ describe('ToolService - Action Capability Gating', () => {
actionsEnabled: true,
});
expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' });
expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' });
});
it('blocks decrypted persisted action secrets before execution-time domain work', async () => {

View file

@ -165,7 +165,7 @@ export interface ResumeContentProtectionDependencies {
getFiles: ResumeContentInspectionInput['getFiles'];
getAgent: (filter: { id: string }) => Promise<Agent | null | undefined>;
getActions: (
filter: { agent_id: { $in: string[] } },
query: { agentId: string[] },
includeSensitive: boolean,
) => Promise<ResumeAction[] | null | undefined>;
getUserMemories: (params: { userId: string; agentId?: string }) => Promise<MemoryContentInput[]>;
@ -642,7 +642,7 @@ async function getResumeActionSnapshots(
ENCRYPTED_ACTION_METADATA_FIELDS,
);
const actions =
(await dependencies.getActions({ agent_id: { $in: agentIds } }, needsEncryptedMetadata)) ?? [];
(await dependencies.getActions({ agentId: agentIds }, needsEncryptedMetadata)) ?? [];
const withFunctions = actions.map((action) => {
const rawSpec = action.metadata?.raw_spec;
if (typeof rawSpec !== 'string') {

View file

@ -301,9 +301,11 @@ describe('createImageAuthorizationMiddleware', () => {
expect(next).toHaveBeenCalledTimes(1);
expect(deps.getAssistant).toHaveBeenCalledWith(
{
'avatar.filepath': {
$in: [assistantPath, `${assistantPath}?manual=false`, `${assistantPath}?manual=true`],
},
avatarFilepath: [
assistantPath,
`${assistantPath}?manual=false`,
`${assistantPath}?manual=true`,
],
},
{ _id: 1, assistant_id: 1, endpoint: 1 },
);

View file

@ -8,7 +8,7 @@ import {
runAsSystem,
tenantStorage,
} from '@librechat/data-schemas';
import type { IAgent, IAssistant, SystemCapability } from '@librechat/data-schemas';
import type { IAgent, IAssistant, AssistantQuery, SystemCapability } from '@librechat/data-schemas';
import type { Request, Response, NextFunction, RequestHandler } from 'express';
import type { FilterQuery, ProjectionType, Types } from 'mongoose';
@ -72,7 +72,7 @@ export interface ImageAuthorizationDeps {
projection?: ProjectionType<IAgent>,
) => Promise<Pick<IAgent, '_id'> | null>;
getAssistant: (
query: FilterQuery<IAssistant>,
query: AssistantQuery,
projection?: ProjectionType<IAssistant>,
) => Promise<AssistantImageRecord | null>;
getImageConfig?: (params: { userId: string; user: ImageUser }) => Promise<{
@ -334,7 +334,7 @@ async function canViewAssistantAvatar(
deps: ImageAuthorizationDeps,
): Promise<boolean> {
const assistant = await deps.getAssistant(
{ 'avatar.filepath': { $in: getStoredPathCandidates(imagePath.canonicalPath) } },
{ avatarFilepath: getStoredPathCandidates(imagePath.canonicalPath) },
{ _id: 1, assistant_id: 1, endpoint: 1 },
);
if (!assistant) {

View file

@ -0,0 +1,125 @@
import mongoose from 'mongoose';
import { MongoMemoryServer } from 'mongodb-memory-server';
import type { IAction } from '~/types';
import { createActionMethods } from './action';
import { createModels } from '~/models';
jest.mock('~/config/winston', () => ({
error: jest.fn(),
warn: jest.fn(),
info: jest.fn(),
debug: jest.fn(),
}));
let mongoServer: InstanceType<typeof MongoMemoryServer>;
let methods: ReturnType<typeof createActionMethods>;
let Action: mongoose.Model<IAction>;
const userA = new mongoose.Types.ObjectId();
const userB = new mongoose.Types.ObjectId();
const seed = (overrides: Partial<IAction>) =>
Action.create({
user: userA,
action_id: 'act-1',
type: 'action_prototype',
metadata: { domain: 'example.com', auth: { type: 'none', token_exchange_method: null } },
...overrides,
} as Partial<IAction>);
beforeAll(async () => {
mongoServer = await MongoMemoryServer.create();
await mongoose.connect(mongoServer.getUri());
Object.assign(mongoose.models, createModels(mongoose));
Action = mongoose.models.Action;
methods = createActionMethods(mongoose);
});
afterAll(async () => {
await mongoose.disconnect();
await mongoServer.stop();
});
beforeEach(async () => {
await Action.deleteMany({});
});
describe('action criteria', () => {
it('finds by a single action id', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-2', agent_id: 'agent-1' });
const found = await methods.getActions({ actionId: 'act-1' }, true);
expect(found.map((a) => a.action_id)).toEqual(['act-1']);
});
it('finds by any of several ids', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-2', agent_id: 'agent-1' });
await seed({ action_id: 'act-3', agent_id: 'agent-1' });
const found = await methods.getActions({ actionId: ['act-1', 'act-3'] }, true);
expect(found.map((a) => a.action_id).sort()).toEqual(['act-1', 'act-3']);
});
it('combines criteria with AND', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-1', agent_id: 'agent-2' });
const found = await methods.getActions({ actionId: 'act-1', agentId: 'agent-2' }, true);
expect(found).toHaveLength(1);
expect(found[0].agent_id).toBe('agent-2');
});
it('scopes deletes to the owning user', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1', user: userA });
await seed({ action_id: 'act-2', agent_id: 'agent-1', user: userB });
const deleted = await methods.deleteActions({ agentId: 'agent-1', user: String(userB) });
expect(deleted).toBe(1);
const remaining = await methods.getActions({}, true);
expect(remaining.map((a) => a.action_id)).toEqual(['act-1']);
});
it('rejects an unrecognized criterion instead of matching every action', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await expect(
methods.getActions({ action_id: 'act-1' } as unknown as { actionId?: string }, true),
).rejects.toThrow("Unknown query criterion: 'action_id'");
});
it('does not delete every action when given an unrecognized criterion', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-2', agent_id: 'agent-2' });
await expect(
methods.deleteActions({ agent_id: 'agent-1' } as unknown as { agentId?: string }),
).rejects.toThrow("Unknown query criterion: 'agent_id'");
await expect(Action.countDocuments({})).resolves.toBe(2);
});
it('does not delete every action when a misspelled criterion carries undefined', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-2', agent_id: 'agent-2' });
const absent: string | undefined = undefined;
await expect(
methods.deleteActions({ agent_id: absent } as unknown as { agentId?: string }),
).rejects.toThrow("Unknown query criterion: 'agent_id'");
await expect(Action.countDocuments({})).resolves.toBe(2);
});
it('still omits a recognized criterion the caller left undefined', async () => {
await seed({ action_id: 'act-1', agent_id: 'agent-1' });
await seed({ action_id: 'act-2', agent_id: 'agent-2' });
const found = await methods.getActions({ actionId: 'act-1', agentId: undefined }, true);
expect(found.map((a) => a.action_id)).toEqual(['act-1']);
});
});

View file

@ -1,42 +1,46 @@
import type { FilterQuery, Model } from 'mongoose';
import type { IAction } from '~/types';
import type { ActionQuery, IAction } from '~/types';
import { buildFilter, type FieldMap } from '~/utils/criteria';
const sensitiveFields = ['api_key', 'oauth_client_id', 'oauth_client_secret'] as const;
const ACTION_FIELDS: FieldMap<ActionQuery> = {
actionId: 'action_id',
agentId: 'agent_id',
assistantId: 'assistant_id',
user: 'user',
};
/** Translates domain action criteria into a Mongo filter. */
function actionFilter(query: ActionQuery): FilterQuery<IAction> {
return buildFilter<ActionQuery, FilterQuery<IAction>>(query, ACTION_FIELDS);
}
export function createActionMethods(mongoose: typeof import('mongoose')): {
getActions: (
searchParams: FilterQuery<IAction>,
includeSensitive?: boolean,
) => Promise<IAction[]>;
updateAction: (
searchParams: FilterQuery<IAction>,
updateData: Partial<IAction>,
) => Promise<IAction | null>;
deleteAction: (searchParams: FilterQuery<IAction>) => Promise<IAction | null>;
deleteActions: (searchParams: FilterQuery<IAction>) => Promise<number>;
getActions: (query: ActionQuery, includeSensitive?: boolean) => Promise<IAction[]>;
updateAction: (query: ActionQuery, updateData: Partial<IAction>) => Promise<IAction | null>;
deleteAction: (query: ActionQuery) => Promise<IAction | null>;
deleteActions: (query: ActionQuery) => Promise<number>;
} {
/**
* Update an action with new data without overwriting existing properties,
* or create a new action if it doesn't exist.
*/
async function updateAction(
searchParams: FilterQuery<IAction>,
query: ActionQuery,
updateData: Partial<IAction>,
): Promise<IAction | null> {
const Action = mongoose.models.Action as Model<IAction>;
const options = { new: true, upsert: true };
return await Action.findOneAndUpdate(searchParams, updateData, options).lean<IAction>();
return await Action.findOneAndUpdate(actionFilter(query), updateData, options).lean<IAction>();
}
/**
* Retrieves all actions that match the given search parameters.
* Retrieves all actions that match the given criteria.
*/
async function getActions(
searchParams: FilterQuery<IAction>,
includeSensitive = false,
): Promise<IAction[]> {
async function getActions(query: ActionQuery, includeSensitive = false): Promise<IAction[]> {
const Action = mongoose.models.Action as Model<IAction>;
const actions = await Action.find(searchParams).lean<IAction[]>();
const actions = await Action.find(actionFilter(query)).lean<IAction[]>();
if (!includeSensitive) {
for (let i = 0; i < actions.length; i++) {
@ -57,19 +61,19 @@ export function createActionMethods(mongoose: typeof import('mongoose')): {
}
/**
* Deletes an action by params.
* Deletes an action by criteria.
*/
async function deleteAction(searchParams: FilterQuery<IAction>): Promise<IAction | null> {
async function deleteAction(query: ActionQuery): Promise<IAction | null> {
const Action = mongoose.models.Action as Model<IAction>;
return await Action.findOneAndDelete(searchParams).lean<IAction>();
return await Action.findOneAndDelete(actionFilter(query)).lean<IAction>();
}
/**
* Deletes actions by params.
* Deletes actions by criteria.
*/
async function deleteActions(searchParams: FilterQuery<IAction>): Promise<number> {
async function deleteActions(query: ActionQuery): Promise<number> {
const Action = mongoose.models.Action as Model<IAction>;
const result = await Action.deleteMany(searchParams);
const result = await Action.deleteMany(actionFilter(query));
return result.deletedCount;
}

View file

@ -8,7 +8,7 @@ import {
} from 'librechat-data-provider';
import type { FilterQuery, Model, PipelineStage, ProjectionType, Types } from 'mongoose';
import type { AgentToolResources } from 'librechat-data-provider';
import type { IAgent, IAclEntry } from '~/types';
import type { IAgent, IAclEntry, ActionQuery } from '~/types';
import { withCodeEnvironmentReference } from './codeEnvironment';
import { filterExistingSkillIds } from './skill';
import logger from '~/config/winston';
@ -116,10 +116,7 @@ export interface AgentDeps {
/** Removes all ACL permissions for a resource. Injected from PermissionService. */
removeAllPermissions: (params: { resourceType: string; resourceId: unknown }) => Promise<void>;
/** Gets actions. Created by createActionMethods. */
getActions: (
searchParams: FilterQuery<unknown>,
includeSensitive?: boolean,
) => Promise<unknown[]>;
getActions: (query: ActionQuery, includeSensitive?: boolean) => Promise<unknown[]>;
/** Returns resource IDs solely owned by the given user. From createAclEntryMethods. */
getSoleOwnedResourceIds: (
userObjectId: Types.ObjectId,
@ -802,7 +799,7 @@ export function createAgentMethods(
if (actionIds.length > 0) {
try {
const actions = await getActions({ action_id: { $in: actionIds } }, true);
const actions = await getActions({ actionId: actionIds }, true);
actionsHash = await generateActionMetadataHash(
currentAgent.actions,

View file

@ -1,20 +1,32 @@
import type { FilterQuery, Model, ProjectionType } from 'mongoose';
import type { IAssistant } from '~/types';
import type { AssistantQuery, IAssistant } from '~/types';
import { buildFilter, type FieldMap } from '~/utils/criteria';
import { createIndexesWithRetry } from '~/utils/retry';
const ASSISTANT_FIELDS: FieldMap<AssistantQuery> = {
assistantId: 'assistant_id',
user: 'user',
avatarFilepath: 'avatar.filepath',
};
/** Translates domain assistant criteria into a Mongo filter. */
function assistantFilter(query: AssistantQuery): FilterQuery<IAssistant> {
return buildFilter<AssistantQuery, FilterQuery<IAssistant>>(query, ASSISTANT_FIELDS);
}
export function createAssistantMethods(mongoose: typeof import('mongoose')): {
updateAssistantDoc: (
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
updateData: Partial<IAssistant>,
) => Promise<IAssistant | null>;
deleteAssistant: (searchParams: FilterQuery<IAssistant>) => Promise<IAssistant | null>;
deleteAssistants: (searchParams: FilterQuery<IAssistant>) => Promise<number>;
deleteAssistant: (query: AssistantQuery) => Promise<IAssistant | null>;
deleteAssistants: (query: AssistantQuery) => Promise<number>;
getAssistants: (
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
select?: string | Record<string, number> | null,
) => Promise<IAssistant[]>;
getAssistant: (
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
projection?: ProjectionType<IAssistant>,
) => Promise<IAssistant | null>;
ensureAssistantIndexes: () => Promise<void>;
@ -37,50 +49,50 @@ export function createAssistantMethods(mongoose: typeof import('mongoose')): {
* or create a new assistant if it doesn't exist.
*/
async function updateAssistantDoc(
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
updateData: Partial<IAssistant>,
): Promise<IAssistant | null> {
const options = { new: true, upsert: true };
return await Assistant().findOneAndUpdate(searchParams, updateData, options).lean<IAssistant>();
return await Assistant()
.findOneAndUpdate(assistantFilter(query), updateData, options)
.lean<IAssistant>();
}
/**
* Retrieves an assistant document based on the provided search params.
*/
async function getAssistant(
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
projection?: ProjectionType<IAssistant>,
): Promise<IAssistant | null> {
await ensureAssistantIndexes();
return await Assistant().findOne(searchParams, projection).lean<IAssistant>();
return await Assistant().findOne(assistantFilter(query), projection).lean<IAssistant>();
}
/**
* Retrieves all assistants that match the given search parameters.
*/
async function getAssistants(
searchParams: FilterQuery<IAssistant>,
query: AssistantQuery,
select: string | Record<string, number> | null = null,
): Promise<IAssistant[]> {
const query = Assistant().find(searchParams);
const search = Assistant().find(assistantFilter(query));
return await (select ? query.select(select) : query).lean<IAssistant[]>();
return await (select ? search.select(select) : search).lean<IAssistant[]>();
}
/**
* Deletes an assistant based on the provided search params.
*/
async function deleteAssistant(
searchParams: FilterQuery<IAssistant>,
): Promise<IAssistant | null> {
return await Assistant().findOneAndDelete(searchParams);
async function deleteAssistant(query: AssistantQuery): Promise<IAssistant | null> {
return await Assistant().findOneAndDelete(assistantFilter(query));
}
/**
* Deletes all assistants matching the given search parameters.
*/
async function deleteAssistants(searchParams: FilterQuery<IAssistant>): Promise<number> {
const result = await Assistant().deleteMany(searchParams);
async function deleteAssistants(query: AssistantQuery): Promise<number> {
const result = await Assistant().deleteMany(assistantFilter(query));
return result.deletedCount;
}

View file

@ -18,8 +18,8 @@ describe('assistantSchema', () => {
} as unknown as typeof import('mongoose');
const methods = createAssistantMethods(mongoose);
await methods.getAssistant({ assistant_id: 'assistant-id' });
await methods.getAssistant({ assistant_id: 'assistant-id' });
await methods.getAssistant({ assistantId: 'assistant-id' });
await methods.getAssistant({ assistantId: 'assistant-id' });
expect(createIndexes).toHaveBeenCalledTimes(1);
});

View file

@ -1,4 +1,5 @@
import mongoose, { Document } from 'mongoose';
import type { OneOrMany } from './query';
export interface IAction extends Document {
user: mongoose.Types.ObjectId;
@ -27,3 +28,15 @@ export interface IAction extends Document {
};
tenantId?: string;
}
/**
* Domain criteria for locating actions. Fields are combined with AND; an array
* value matches any of its entries. Deliberately storage-agnostic — it names
* domain concepts, not stored field names or query operators.
*/
export interface ActionQuery {
actionId?: OneOrMany<string>;
agentId?: OneOrMany<string>;
assistantId?: OneOrMany<string>;
user?: string;
}

View file

@ -1,4 +1,5 @@
import { Document, Types } from 'mongoose';
import type { OneOrMany } from './query';
export interface IAssistant extends Document {
user: Types.ObjectId;
@ -15,3 +16,14 @@ export interface IAssistant extends Document {
append_current_datetime?: boolean;
tenantId?: string;
}
/**
* Domain criteria for locating assistants. Fields are combined with AND; an
* array value matches any of its entries.
*/
export interface AssistantQuery {
assistantId?: OneOrMany<string>;
user?: string;
/** Matches the stored avatar path, used to authorize avatar image reads. */
avatarFilepath?: OneOrMany<string>;
}

View file

@ -20,6 +20,7 @@ export * from './agentApiKey';
export * from './agentCategory';
export * from './codeEnvironment';
export * from './role';
export * from './query';
export * from './action';
export * from './assistant';
export * from './file';

View file

@ -0,0 +1,7 @@
/**
* A criterion that matches a single value or any one of several.
*
* Storage-agnostic on purpose: it is part of the domain query vocabulary, not
* of any engine's filter language.
*/
export type OneOrMany<T> = T | T[];

View file

@ -0,0 +1,130 @@
import { buildFilter, matchAny, type FieldMap } from './criteria';
type SampleQuery = {
actionId?: string | string[];
agentId?: string | string[];
user?: string;
};
const FIELDS: FieldMap<SampleQuery> = {
actionId: 'action_id',
agentId: 'agent_id',
user: 'user',
};
describe('matchAny', () => {
it('passes a scalar through unchanged', () => {
expect(matchAny('abc')).toBe('abc');
});
it('translates a list into an $in expression', () => {
expect(matchAny(['a', 'b'])).toEqual({ $in: ['a', 'b'] });
});
it('translates an empty list into an $in that matches nothing', () => {
expect(matchAny([])).toEqual({ $in: [] });
});
});
describe('buildFilter', () => {
it('maps criteria onto their stored field names', () => {
expect(buildFilter<SampleQuery, Record<string, unknown>>({ actionId: 'a1' }, FIELDS)).toEqual({
action_id: 'a1',
});
});
it('combines multiple criteria', () => {
const filter = buildFilter<SampleQuery, Record<string, unknown>>(
{ agentId: 'agent-1', user: 'user-1' },
FIELDS,
);
expect(filter).toEqual({ agent_id: 'agent-1', user: 'user-1' });
});
it('translates list criteria into $in', () => {
const filter = buildFilter<SampleQuery, Record<string, unknown>>(
{ agentId: ['a', 'b'] },
FIELDS,
);
expect(filter).toEqual({ agent_id: { $in: ['a', 'b'] } });
});
it('omits criteria the caller left undefined', () => {
const filter = buildFilter<SampleQuery, Record<string, unknown>>(
{ actionId: 'a1', agentId: undefined },
FIELDS,
);
expect(filter).toEqual({ action_id: 'a1' });
});
it('builds an empty filter from an empty query, so "list all" stays expressible', () => {
expect(buildFilter<SampleQuery, Record<string, unknown>>({}, FIELDS)).toEqual({});
});
it('throws on an unrecognized criterion rather than silently widening the filter', () => {
const query = { assistant_id: 'asst_1' } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Unknown query criterion: 'assistant_id'",
);
});
it('throws even when a recognized criterion is also present', () => {
const query = { user: 'user-1', id: 'asst_1' } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Unknown query criterion: 'id'",
);
});
it('throws on an unrecognized criterion whose value is undefined', () => {
/* A JS caller misspelling a criterion whose value is absent must not slip
through the undefined-omission rule and produce a filter matching everything. */
const query = { agent_id: undefined } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Unknown query criterion: 'agent_id'",
);
});
it('throws on a criterion that only resolves through the field map prototype', () => {
const query = { toString: 'x' } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Unknown query criterion: 'toString'",
);
});
it('refuses a Mongo operator object smuggled in as a criterion value', () => {
const query = { agentId: { $ne: null } } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Invalid value for query criterion 'agentId'",
);
});
it('refuses an operator object hidden inside a list criterion', () => {
const query = { agentId: ['a', { $ne: null }] } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Invalid value for query criterion 'agentId'",
);
});
it('refuses null, which would match documents missing the field', () => {
const query = { user: null } as unknown as SampleQuery;
expect(() => buildFilter<SampleQuery, Record<string, unknown>>(query, FIELDS)).toThrow(
"Invalid value for query criterion 'user'",
);
});
it('accepts the scalar shapes a domain criterion may carry', () => {
type Mixed = { actionId?: string; count?: number; active?: boolean; since?: Date };
const fields: FieldMap<Mixed> = {
actionId: 'action_id',
count: 'count',
active: 'active',
since: 'since',
};
const since = new Date('2026-01-01T00:00:00.000Z');
const filter = buildFilter<Mixed, Record<string, unknown>>(
{ actionId: 'a1', count: 2, active: true, since },
fields,
);
expect(filter).toEqual({ action_id: 'a1', count: 2, active: true, since });
});
});

View file

@ -0,0 +1,78 @@
import type { OneOrMany } from '~/types/query';
/** Maps every criterion of a domain query to the field it is stored under. */
export type FieldMap<TQuery> = Readonly<Record<keyof TQuery & string, string>>;
/** The value shapes a domain criterion may carry. */
type CriterionValue = string | number | boolean | Date;
function isCriterionValue(value: unknown): value is CriterionValue {
const type = typeof value;
return type === 'string' || type === 'number' || type === 'boolean' || value instanceof Date;
}
/**
* Rejects anything that is not a scalar or a list of scalars.
*
* A criterion is a domain value, never a query fragment. Without this a
* JavaScript caller could pass `{ agentId: { $ne: null } }` and have the
* operator copied verbatim into the filter, widening the scope the field map
* was supposed to fix.
*/
function assertCriterionValue(
key: string,
value: unknown,
): asserts value is OneOrMany<CriterionValue> {
const values = Array.isArray(value) ? value : [value];
for (const entry of values) {
if (!isCriterionValue(entry)) {
throw new Error(
`Invalid value for query criterion '${key}': expected a scalar or array of scalars`,
);
}
}
}
/**
* Translates a scalar-or-list criterion into a Mongo match expression. Domain
* query types express "any of these" as an array; the engine spells it.
*/
export function matchAny<T>(value: OneOrMany<T>): T | { $in: T[] } {
return Array.isArray(value) ? { $in: value } : value;
}
/**
* Builds a Mongo filter from a domain query, omitting criteria the caller left
* undefined.
*
* Throws on any criterion the field map does not cover, and on any value that
* is not a scalar or list of scalars. Callers in `api/` are JavaScript and get
* no compile-time checking, and a silently accepted bad criterion would widen
* the filter — an unscoped `findOne` returns an arbitrary document rather than
* none, and an unscoped `deleteMany` removes every document — so this fails
* closed instead.
*
* Keys are validated before the undefined-omission rule is applied: a
* misspelled criterion that happens to carry `undefined` must still be
* rejected, not quietly skipped. The lookup is an own-property check so that
* inherited names like `toString` cannot resolve through the field map's
* prototype.
*/
export function buildFilter<TQuery extends object, TFilter>(
query: TQuery,
fields: FieldMap<TQuery>,
): TFilter {
const filter: Record<string, unknown> = {};
for (const key of Object.keys(query) as Array<keyof TQuery & string>) {
if (!Object.prototype.hasOwnProperty.call(fields, key)) {
throw new Error(`Unknown query criterion: '${key}'`);
}
const value = query[key];
if (value === undefined) {
continue;
}
assertCriterionValue(key, value);
filter[fields[key]] = matchAny(value);
}
return filter as TFilter;
}