🧠 fix: Return 400 for Invalid Memory Keys (#14618)

* fix: return 400 for invalid memory key format instead of 500

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: trim PATCH rename key to match POST normalization

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: return 400 when PATCH rename key is not a string

A non-string `key` in the PATCH body (e.g. {"key": 123}) previously fell
back to the URL key and updated the memory with a 200, hiding the invalid
field. Reject any defined non-string key with 400 before normalization.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: share memory key validation between schema and routes

* fix: reject blank memory rename keys

* test: use real data-provider exports in memory route suite

* fix: share memory key validation with ID updates

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com>

api/server/routes/__tests__/memories.spec.js

@@ -0,0 +1,165 @@
+const express = require('express');
+const request = require('supertest');
+
+jest.mock('@librechat/api', () => ({
+  ...jest.requireActual('@librechat/api'),
+  Tokenizer: {
+    getTokenCount: jest.fn().mockReturnValue(5),
+  },
+  generateCheckAccess: jest.fn(() => (req, res, next) => next()),
+}));
+
+jest.mock('librechat-data-provider', () => ({
+  ...jest.requireActual('librechat-data-provider'),
+}));
+
+jest.mock('~/models', () => ({
+  getAllUserMemories: jest.fn(),
+  getUserMemories: jest.fn(),
+  toggleUserMemories: jest.fn(),
+  getRoleByName: jest.fn(),
+  createMemory: jest.fn(),
+  deleteMemory: jest.fn(),
+  setMemory: jest.fn(),
+  getAgents: jest.fn(),
+}));
+
+jest.mock('~/server/services/PermissionService', () => ({
+  findAccessibleResources: jest.fn().mockResolvedValue([]),
+}));
+
+jest.mock('~/server/middleware', () => ({
+  requireJwtAuth: (req, res, next) => next(),
+  configMiddleware: (req, res, next) => next(),
+}));
+
+const { createMemory, getUserMemories, setMemory } = require('~/models');
+
+describe('memories routes', () => {
+  let app;
+
+  beforeAll(() => {
+    const memoriesRouter = require('../memories');
+
+    app = express();
+    app.use(express.json());
+    app.use((req, res, next) => {
+      req.user = { id: 'user-1', role: 'USER' };
+      next();
+    });
+    app.use('/api/memories', memoriesRouter);
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('POST /', () => {
+    it('returns 400 when the key contains invalid characters', async () => {
+      const response = await request(app)
+        .post('/api/memories')
+        .send({ key: 'My Key!', value: 'some value' });
+
+      expect(response.status).toBe(400);
+      expect(response.body.error).toContain('lowercase letters and underscores');
+      expect(createMemory).not.toHaveBeenCalled();
+    });
+
+    it('returns 400 when the key contains uppercase letters', async () => {
+      const response = await request(app)
+        .post('/api/memories')
+        .send({ key: 'myKey', value: 'some value' });
+
+      expect(response.status).toBe(400);
+      expect(response.body.error).toContain('lowercase letters and underscores');
+      expect(createMemory).not.toHaveBeenCalled();
+    });
+
+    it('creates a memory when the key is valid', async () => {
+      getUserMemories
+        .mockResolvedValueOnce([])
+        .mockResolvedValueOnce([{ key: 'my_key', value: 'some value', tokenCount: 5 }]);
+      createMemory.mockResolvedValue({ ok: true });
+
+      const response = await request(app)
+        .post('/api/memories')
+        .send({ key: 'my_key', value: 'some value' });
+
+      expect(response.status).toBe(201);
+      expect(response.body.created).toBe(true);
+      expect(createMemory).toHaveBeenCalledWith(
+        expect.objectContaining({ userId: 'user-1', key: 'my_key' }),
+      );
+    });
+  });
+
+  describe('PATCH /:key', () => {
+    it('returns 400 when renaming to a key with invalid characters', async () => {
+      const response = await request(app)
+        .patch('/api/memories/my_key')
+        .send({ key: 'New Key', value: 'updated value' });
+
+      expect(response.status).toBe(400);
+      expect(response.body.error).toContain('lowercase letters and underscores');
+      expect(createMemory).not.toHaveBeenCalled();
+    });
+
+    it('returns 400 when the key is not a string', async () => {
+      const response = await request(app)
+        .patch('/api/memories/my_key')
+        .send({ key: 123, value: 'updated value' });
+
+      expect(response.status).toBe(400);
+      expect(response.body.error).toBe('Key must be a string.');
+      expect(setMemory).not.toHaveBeenCalled();
+      expect(createMemory).not.toHaveBeenCalled();
+    });
+
+    it('rejects a whitespace-only rename instead of updating the existing memory', async () => {
+      getUserMemories.mockResolvedValue([{ key: 'my_key', value: 'old value', tokenCount: 5 }]);
+      setMemory.mockResolvedValue({ ok: true });
+
+      const response = await request(app)
+        .patch('/api/memories/my_key')
+        .send({ key: ' \t\n ', value: 'updated value' });
+
+      expect(response.status).toBe(400);
+      expect(setMemory).not.toHaveBeenCalled();
+      expect(createMemory).not.toHaveBeenCalled();
+    });
+
+    it('trims the new key before validating and renaming', async () => {
+      const { deleteMemory } = require('~/models');
+      getUserMemories
+        .mockResolvedValueOnce([{ key: 'my_key', value: 'old value', tokenCount: 5 }])
+        .mockResolvedValueOnce([{ key: 'new_key', value: 'updated value', tokenCount: 5 }]);
+      createMemory.mockResolvedValue({ ok: true });
+      deleteMemory.mockResolvedValue({ ok: true });
+
+      const response = await request(app)
+        .patch('/api/memories/my_key')
+        .send({ key: '  new_key  ', value: 'updated value' });
+
+      expect(response.status).toBe(200);
+      expect(createMemory).toHaveBeenCalledWith(expect.objectContaining({ key: 'new_key' }));
+      expect(deleteMemory).toHaveBeenCalledWith(expect.objectContaining({ key: 'my_key' }));
+    });
+
+    it('updates the value when the key is unchanged', async () => {
+      getUserMemories
+        .mockResolvedValueOnce([{ key: 'my_key', value: 'old value', tokenCount: 5 }])
+        .mockResolvedValueOnce([{ key: 'my_key', value: 'updated value', tokenCount: 5 }]);
+      setMemory.mockResolvedValue({ ok: true });
+
+      const response = await request(app)
+        .patch('/api/memories/my_key')
+        .send({ value: 'updated value' });
+
+      expect(response.status).toBe(200);
+      expect(response.body.updated).toBe(true);
+      expect(setMemory).toHaveBeenCalledWith(
+        expect.objectContaining({ userId: 'user-1', key: 'my_key' }),
+      );
+    });
+  });
+});

api/server/routes/memories.js

@@ -1,4 +1,5 @@
 const express = require('express');
+const { isValidMemoryKey } = require('@librechat/data-schemas');
 const {
   Tokenizer,
   generateCheckAccess,
@@ -188,6 +189,12 @@ router.post('/', createMemoryMiddleware, async (req, res) => {
     return res.status(400).json({ error: 'Value is required and must be a non-empty string.' });
   }
 
+  if (!isValidMemoryKey(key.trim())) {
+    return res.status(400).json({
+      error: 'Key must only contain lowercase letters and underscores.',
+    });
+  }
+
   const appConfig = req.config;
   const memoryConfig = appConfig?.memory;
   const charLimit = memoryConfig?.charLimit || 10000;
@@ -315,7 +322,18 @@ router.patch('/:key', updateMemoryMiddleware, async (req, res) => {
     return res.status(400).json({ error: 'Value is required and must be a non-empty string.' });
   }
 
-  const newKey = bodyKey || urlKey;
+  if (bodyKey !== undefined && typeof bodyKey !== 'string') {
+    return res.status(400).json({ error: 'Key must be a string.' });
+  }
+
+  const newKey = bodyKey === undefined ? urlKey : bodyKey.trim();
+
+  if (newKey !== urlKey && !isValidMemoryKey(newKey)) {
+    return res.status(400).json({
+      error: 'Key must only contain lowercase letters and underscores.',
+    });
+  }
+
   const appConfig = req.config;
   const memoryConfig = appConfig?.memory;
   const charLimit = memoryConfig?.charLimit || 10000;

api/server/routes/memories.test.js

@@ -55,19 +55,6 @@ jest.mock('@librechat/api', () => ({
   contentFilterBlockResponse: jest.fn(),
 }));
 
-jest.mock('librechat-data-provider', () => ({
-  PermissionTypes: { MEMORIES: 'MEMORIES' },
-  Permissions: {
-    USE: 'USE',
-    READ: 'READ',
-    CREATE: 'CREATE',
-    UPDATE: 'UPDATE',
-    OPT_OUT: 'OPT_OUT',
-  },
-  ResourceType: { AGENT: 'agent' },
-  PermissionBits: { VIEW: 1 },
-}));
-
 jest.mock('~/server/services/PermissionService', () => ({
   checkPermission: (...args) => mockCheckPermission(...args),
   findAccessibleResources: jest.fn().mockResolvedValue([]),

packages/api/src/memory/handlers.ts

@@ -1,5 +1,7 @@
-import { hasActivePiiPatterns, type FiltersConfig } from 'librechat-data-provider';
+import { isValidMemoryKey } from '@librechat/data-schemas';
+import { hasActivePiiPatterns } from 'librechat-data-provider';
 import type { MemoryMethods } from '@librechat/data-schemas';
+import type { FiltersConfig } from 'librechat-data-provider';
 import type { Request, Response } from 'express';
 import type { MemoryContentInput } from '../protection/adapters/submissions';
 import type { ProjectedStoredMemory } from './protection';
@@ -90,7 +92,7 @@ export function createMemoryManagementHandlers(deps: MemoryManagementHandlersDep
         error: `Key exceeds maximum length of 1000 characters. Current length: ${key.length} characters.`,
       });
     }
-    if (key != null && !/^[a-z_]+$/.test(key)) {
+    if (key != null && !isValidMemoryKey(key)) {
       return res
         .status(400)
         .json({ error: 'Key must only contain lowercase letters and underscores.' });

packages/data-schemas/src/schema/index.ts

@@ -29,7 +29,7 @@ export { default as tokenSchema } from './token';
 export { default as toolCallSchema } from './toolCall';
 export { default as transactionSchema } from './transaction';
 export { default as userSchema } from './user';
-export { default as memorySchema } from './memory';
+export { default as memorySchema, isValidMemoryKey } from './memory';
 export { default as toolFavoriteSchema } from './favorite';
 export { default as groupSchema } from './group';
 export { default as systemGrantSchema } from './systemGrant';

packages/data-schemas/src/schema/memory.ts

@@ -1,6 +1,12 @@
 import { Schema } from 'mongoose';
 import type { IMemoryEntry } from '~/types/memory';
 
+const MEMORY_KEY_REGEX = /^[a-z_]+$/;
+
+export function isValidMemoryKey(key: string): boolean {
+  return MEMORY_KEY_REGEX.test(key);
+}
+
 const MemoryEntrySchema: Schema<IMemoryEntry> = new Schema({
   userId: {
     type: Schema.Types.ObjectId,
@@ -12,7 +18,7 @@ const MemoryEntrySchema: Schema<IMemoryEntry> = new Schema({
     type: String,
     required: true,
     validate: {
-      validator: (v: string) => /^[a-z_]+$/.test(v),
+      validator: isValidMemoryKey,
       message: 'Key must only contain lowercase letters and underscores',
     },
   },