mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-28 12:44:28 +00:00
🛡️ fix: Address Codex re-review on memory capability (round 2)
- Strip the memory capability from the FINAL served agents config, not just defaults; loadEndpoints reparses any endpoints.agents block, so memory was still exposed in that common shape (packages/data-schemas/src/app/service.ts) + regression test. - Re-check the full memory gate (config, opt-out, MEMORIES.USE) inside handleTools before constructing set_memory/delete_memory, so an unsolicited tool call from a model/custom endpoint can't bypass the runtime gates (api/app/clients/tools/util/handleTools.js). - Restore the persisted memory toggle for model-spec conversations via applyModelSpecEphemeralAgent (client/src/utils/endpoints.ts). - Clear LAST_MEMORY_TOGGLE_ on logout and clear-all-chats so a stale memory preference can't leak across users on a shared browser (client/src/utils/localStorage.ts).
This commit is contained in:
parent
ccca0ad1d5
commit
8f9e6d9e13
5 changed files with 67 additions and 5 deletions
|
|
@ -3,6 +3,7 @@ const { Calculator, createSearchTool, createCodeExecutionTool } = require('@libr
|
|||
const {
|
||||
checkAccess,
|
||||
toolkitParent,
|
||||
isMemoryEnabled,
|
||||
createSafeUser,
|
||||
mcpToolPattern,
|
||||
createMemoryTool,
|
||||
|
|
@ -54,6 +55,34 @@ const { getMCPServerTools } = require('~/server/services/Config');
|
|||
const { getMCPServersRegistry } = require('~/config');
|
||||
const { getRoleByName, setMemory, deleteMemory, getFormattedMemories } = require('~/models');
|
||||
|
||||
/**
|
||||
* Re-checks the full memory gate before constructing inline memory tools.
|
||||
* The event-driven executor loads tools by name, so an unsolicited
|
||||
* `set_memory`/`delete_memory` call must not bypass the config, opt-out, and
|
||||
* permission checks enforced when the tools were registered.
|
||||
* @param {ServerRequest} [req]
|
||||
* @returns {Promise<boolean>}
|
||||
*/
|
||||
async function isMemoryToolUsable(req) {
|
||||
if (!isMemoryEnabled(req?.config?.memory)) {
|
||||
return false;
|
||||
}
|
||||
if (req?.user?.personalization?.memories === false) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
return await checkAccess({
|
||||
user: req.user,
|
||||
permissionType: PermissionTypes.MEMORIES,
|
||||
permissions: [Permissions.USE],
|
||||
getRoleByName,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.error('[handleTools] Memory permission check failed:', error);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates the availability and authentication of tools for a user based on environment variables or user-specific plugin authentication values.
|
||||
* Tools without required authentication or with valid authentication are considered valid.
|
||||
|
|
@ -366,6 +395,9 @@ const loadTools = async ({
|
|||
const validKeys = memoryConfig?.validKeys;
|
||||
const tokenLimit = memoryConfig?.tokenLimit;
|
||||
requestedTools[tool] = async () => {
|
||||
if (!(await isMemoryToolUsable(options.req))) {
|
||||
return null;
|
||||
}
|
||||
let totalTokens = 0;
|
||||
if (tokenLimit) {
|
||||
try {
|
||||
|
|
@ -381,6 +413,9 @@ const loadTools = async ({
|
|||
} else if (tool === DELETE_MEMORY_TOOL_NAME) {
|
||||
const memoryConfig = options.req?.config?.memory;
|
||||
requestedTools[tool] = async () => {
|
||||
if (!(await isMemoryToolUsable(options.req))) {
|
||||
return null;
|
||||
}
|
||||
return createDeleteMemoryTool({
|
||||
userId: user,
|
||||
deleteMemory,
|
||||
|
|
|
|||
|
|
@ -380,6 +380,7 @@ export function applyModelSpecEphemeralAgent({
|
|||
['web_search', LocalStorageKeys.LAST_WEB_SEARCH_TOGGLE_],
|
||||
['file_search', LocalStorageKeys.LAST_FILE_SEARCH_TOGGLE_],
|
||||
['artifacts', LocalStorageKeys.LAST_ARTIFACTS_TOGGLE_],
|
||||
['memory', LocalStorageKeys.LAST_MEMORY_TOGGLE_],
|
||||
];
|
||||
|
||||
for (const [toolKey, storagePrefix] of toolStorageMap) {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ export function clearLocalStorage(skipFirst?: boolean) {
|
|||
if (
|
||||
key.startsWith(LocalStorageKeys.LAST_MCP_) ||
|
||||
key.startsWith(LocalStorageKeys.LAST_CODE_TOGGLE_) ||
|
||||
key.startsWith(LocalStorageKeys.LAST_MEMORY_TOGGLE_) ||
|
||||
key.startsWith(LocalStorageKeys.ASST_ID_PREFIX) ||
|
||||
key.startsWith(LocalStorageKeys.AGENT_ID_PREFIX) ||
|
||||
key.startsWith(LocalStorageKeys.LAST_CONVO_SETUP) ||
|
||||
|
|
@ -69,6 +70,7 @@ export function clearAllConversationStorage() {
|
|||
if (
|
||||
key.startsWith(LocalStorageKeys.LAST_MCP_) ||
|
||||
key.startsWith(LocalStorageKeys.LAST_CODE_TOGGLE_) ||
|
||||
key.startsWith(LocalStorageKeys.LAST_MEMORY_TOGGLE_) ||
|
||||
key.startsWith(LocalStorageKeys.TEXT_DRAFT) ||
|
||||
key.startsWith(LocalStorageKeys.ASST_ID_PREFIX) ||
|
||||
key.startsWith(LocalStorageKeys.AGENT_ID_PREFIX) ||
|
||||
|
|
|
|||
|
|
@ -174,4 +174,14 @@ describe('AppService memory capability', () => {
|
|||
AgentCapabilities.memory,
|
||||
);
|
||||
});
|
||||
|
||||
it('strips the memory capability even when an agents endpoint block is configured', async () => {
|
||||
const config = {
|
||||
endpoints: { [EModelEndpoint.agents]: { disableBuilder: true } },
|
||||
} as DeepPartial<TCustomConfig>;
|
||||
const result = await AppService({ config });
|
||||
expect(result.endpoints?.[EModelEndpoint.agents]?.capabilities).not.toContain(
|
||||
AgentCapabilities.memory,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,7 +5,12 @@ import {
|
|||
skillSyncConfigSchema,
|
||||
summarizationConfigSchema,
|
||||
} from 'librechat-data-provider';
|
||||
import type { TCustomConfig, FileSources, DeepPartial } from 'librechat-data-provider';
|
||||
import type {
|
||||
FileSources,
|
||||
DeepPartial,
|
||||
TCustomConfig,
|
||||
TAgentsEndpoint,
|
||||
} from 'librechat-data-provider';
|
||||
import type { AppConfig, FunctionTool } from '~/types/app';
|
||||
import { loadMemoryConfig, isMemoryEnabled } from './memory';
|
||||
import { loadDefaultInterface } from './interface';
|
||||
|
|
@ -162,12 +167,20 @@ export const AppService = async (params?: {
|
|||
/** The `memory` capability only functions when memory is configured and
|
||||
* enabled. Drop it from the served capability set otherwise so the agent
|
||||
* builder toggle, ephemeral badge, and backend capability gate stay
|
||||
* consistent instead of exposing an inert memory toggle. */
|
||||
if (!isMemoryEnabled(memory) && Array.isArray(agentsDefaults.capabilities)) {
|
||||
agentsDefaults.capabilities = agentsDefaults.capabilities.filter(
|
||||
* consistent instead of exposing an inert memory toggle. Applied to the
|
||||
* final served agents config — `loadEndpoints` reparses any
|
||||
* `endpoints.agents` block and would otherwise restore the default
|
||||
* capability. */
|
||||
const memoryDisabled = !isMemoryEnabled(memory);
|
||||
const stripInertMemoryCapability = (agentsEndpoint?: Partial<TAgentsEndpoint>): void => {
|
||||
if (!memoryDisabled || !agentsEndpoint || !Array.isArray(agentsEndpoint.capabilities)) {
|
||||
return;
|
||||
}
|
||||
agentsEndpoint.capabilities = agentsEndpoint.capabilities.filter(
|
||||
(capability) => capability !== AgentCapabilities.memory,
|
||||
);
|
||||
}
|
||||
};
|
||||
stripInertMemoryCapability(agentsDefaults);
|
||||
|
||||
if (!Object.keys(config).length) {
|
||||
const appConfig = {
|
||||
|
|
@ -180,6 +193,7 @@ export const AppService = async (params?: {
|
|||
}
|
||||
|
||||
const loadedEndpoints = loadEndpoints(config, agentsDefaults);
|
||||
stripInertMemoryCapability(loadedEndpoints[EModelEndpoint.agents]);
|
||||
|
||||
const appConfig: AppConfig = {
|
||||
...defaultConfig,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue