🛡️ feat: Add Middleware for JSON Parsing and Prompt Group Updates (#10757)

* 🗨️ fix: Safe Validation for Prompt Updates

- Added `safeValidatePromptGroupUpdate` function to validate and sanitize prompt group update requests, ensuring only allowed fields are processed and sensitive fields are stripped.
- Updated the `patchPromptGroup` route to utilize the new validation function, returning appropriate error messages for invalid requests.
- Introduced comprehensive tests for the validation logic, covering various scenarios including allowed and disallowed fields, enhancing overall request integrity and security.
- Created a new schema file for prompt group updates, defining validation rules and types for better maintainability.

* 🔒 feat: Add JSON parse error handling middleware

api/server/experimental.js

@@ -16,6 +16,7 @@ const {
   isEnabled,
   ErrorController,
   performStartupChecks,
+  handleJsonParseError,
   initializeFileStorage,
 } = require('@librechat/api');
 const { connectDb, indexSync } = require('~/db');
@@ -245,6 +246,7 @@ if (cluster.isMaster) {
     app.use(noIndex);
     app.use(express.json({ limit: '3mb' }));
     app.use(express.urlencoded({ extended: true, limit: '3mb' }));
+    app.use(handleJsonParseError);
     app.use(mongoSanitize());
     app.use(cors());
     app.use(cookieParser());

api/server/index.js

@@ -14,6 +14,7 @@ const {
   isEnabled,
   ErrorController,
   performStartupChecks,
+  handleJsonParseError,
   initializeFileStorage,
 } = require('@librechat/api');
 const { connectDb, indexSync } = require('~/db');
@@ -81,6 +82,7 @@ const startServer = async () => {
   app.use(noIndex);
   app.use(express.json({ limit: '3mb' }));
   app.use(express.urlencoded({ extended: true, limit: '3mb' }));
+  app.use(handleJsonParseError);
   app.use(mongoSanitize());
   app.use(cors());
   app.use(cookieParser());

api/server/routes/prompts.js

@@ -5,6 +5,7 @@ const {
   markPublicPromptGroups,
   buildPromptGroupFilter,
   formatPromptGroupsResponse,
+  safeValidatePromptGroupUpdate,
   createEmptyPromptGroupsResponse,
   filterAccessibleIdsBySharedLogic,
 } = require('@librechat/api');
@@ -344,7 +345,16 @@ const patchPromptGroup = async (req, res) => {
     if (req.user.role === SystemRoles.ADMIN) {
       delete filter.author;
     }
-    const promptGroup = await updatePromptGroup(filter, req.body);
+
+    const validationResult = safeValidatePromptGroupUpdate(req.body);
+    if (!validationResult.success) {
+      return res.status(400).send({
+        error: 'Invalid request body',
+        details: validationResult.error.errors,
+      });
+    }
+
+    const promptGroup = await updatePromptGroup(filter, validationResult.data);
     res.status(200).send(promptGroup);
   } catch (error) {
     logger.error(error);

api/server/routes/prompts.test.js

@@ -544,6 +544,169 @@ describe('Prompt Routes - ACL Permissions', () => {
     });
   });
 
+  describe('PATCH /api/prompts/groups/:groupId - Update Prompt Group Security', () => {
+    let testGroup;
+
+    beforeEach(async () => {
+      // Create a prompt group
+      testGroup = await PromptGroup.create({
+        name: 'Security Test Group',
+        category: 'security-test',
+        author: testUsers.owner._id,
+        authorName: testUsers.owner.name,
+        productionId: new ObjectId(),
+      });
+
+      // Grant owner permissions
+      await grantPermission({
+        principalType: PrincipalType.USER,
+        principalId: testUsers.owner._id,
+        resourceType: ResourceType.PROMPTGROUP,
+        resourceId: testGroup._id,
+        accessRoleId: AccessRoleIds.PROMPTGROUP_OWNER,
+        grantedBy: testUsers.owner._id,
+      });
+    });
+
+    afterEach(async () => {
+      await PromptGroup.deleteMany({});
+      await AclEntry.deleteMany({});
+    });
+
+    it('should allow updating allowed fields (name, category, oneliner)', async () => {
+      const updateData = {
+        name: 'Updated Group Name',
+        category: 'updated-category',
+        oneliner: 'Updated description',
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(updateData)
+        .expect(200);
+
+      expect(response.body.name).toBe(updateData.name);
+      expect(response.body.category).toBe(updateData.category);
+      expect(response.body.oneliner).toBe(updateData.oneliner);
+    });
+
+    it('should reject request with author field (400 Bad Request)', async () => {
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        author: testUsers.noAccess._id.toString(), // Try to change ownership
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+      expect(response.body.details).toBeDefined();
+    });
+
+    it('should reject request with authorName field (400 Bad Request)', async () => {
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        authorName: 'Malicious Author Name',
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+    });
+
+    it('should reject request with _id field (400 Bad Request)', async () => {
+      const newId = new ObjectId();
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        _id: newId.toString(),
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+    });
+
+    it('should reject request with productionId field (400 Bad Request)', async () => {
+      const newProductionId = new ObjectId();
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        productionId: newProductionId.toString(),
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+    });
+
+    it('should reject request with createdAt field (400 Bad Request)', async () => {
+      const maliciousDate = new Date('2020-01-01');
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        createdAt: maliciousDate.toISOString(),
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+    });
+
+    it('should reject request with __v field (400 Bad Request)', async () => {
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        __v: 999,
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected
+      expect(response.body.error).toBe('Invalid request body');
+    });
+
+    it('should reject request with multiple sensitive fields (400 Bad Request)', async () => {
+      const maliciousUpdate = {
+        name: 'Legit Update',
+        author: testUsers.noAccess._id.toString(),
+        authorName: 'Hacker',
+        _id: new ObjectId().toString(),
+        productionId: new ObjectId().toString(),
+        createdAt: new Date('2020-01-01').toISOString(),
+        __v: 999,
+      };
+
+      const response = await request(app)
+        .patch(`/api/prompts/groups/${testGroup._id}`)
+        .send(maliciousUpdate)
+        .expect(400);
+
+      // Verify the request was rejected with validation errors
+      expect(response.body.error).toBe('Invalid request body');
+      expect(response.body.details).toBeDefined();
+      expect(Array.isArray(response.body.details)).toBe(true);
+    });
+  });
+
   describe('Pagination', () => {
     beforeEach(async () => {
       // Create multiple prompt groups for pagination testing

packages/api/src/middleware/index.ts

@@ -1,3 +1,4 @@
 export * from './access';
 export * from './error';
 export * from './balance';
+export * from './json';

packages/api/src/middleware/json.spec.ts

@@ -0,0 +1,158 @@
+import { handleJsonParseError } from './json';
+import type { Request, Response, NextFunction } from 'express';
+
+describe('handleJsonParseError', () => {
+  let req: Partial<Request>;
+  let res: Partial<Response>;
+  let next: NextFunction;
+  let jsonSpy: jest.Mock;
+  let statusSpy: jest.Mock;
+
+  beforeEach(() => {
+    req = {
+      path: '/api/test',
+      method: 'POST',
+      ip: '127.0.0.1',
+    };
+
+    jsonSpy = jest.fn();
+    statusSpy = jest.fn().mockReturnValue({ json: jsonSpy });
+
+    res = {
+      status: statusSpy,
+      json: jsonSpy,
+    };
+
+    next = jest.fn();
+  });
+
+  describe('JSON parse errors', () => {
+    it('should handle JSON SyntaxError with 400 status', () => {
+      const err = new SyntaxError('Unexpected token < in JSON at position 0') as SyntaxError & {
+        status?: number;
+        body?: unknown;
+      };
+      err.status = 400;
+      err.body = {};
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(statusSpy).toHaveBeenCalledWith(400);
+      expect(jsonSpy).toHaveBeenCalledWith({
+        error: 'Invalid JSON format',
+        message: 'The request body contains malformed JSON',
+      });
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should not reflect user input in error message', () => {
+      const maliciousInput = '<script>alert("xss")</script>';
+      const err = new SyntaxError(
+        `Unexpected token < in JSON at position 0: ${maliciousInput}`,
+      ) as SyntaxError & {
+        status?: number;
+        body?: unknown;
+      };
+      err.status = 400;
+      err.body = maliciousInput;
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(statusSpy).toHaveBeenCalledWith(400);
+      const errorResponse = jsonSpy.mock.calls[0][0];
+      expect(errorResponse.message).not.toContain(maliciousInput);
+      expect(errorResponse.message).toBe('The request body contains malformed JSON');
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should handle JSON parse error with HTML tags in body', () => {
+      const err = new SyntaxError('Invalid JSON') as SyntaxError & {
+        status?: number;
+        body?: unknown;
+      };
+      err.status = 400;
+      err.body = '<html><body><h1>XSS</h1></body></html>';
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(statusSpy).toHaveBeenCalledWith(400);
+      const errorResponse = jsonSpy.mock.calls[0][0];
+      expect(errorResponse.message).not.toContain('<html>');
+      expect(errorResponse.message).not.toContain('<script>');
+      expect(next).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('non-JSON errors', () => {
+    it('should pass through non-SyntaxError errors', () => {
+      const err = new Error('Some other error');
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(statusSpy).not.toHaveBeenCalled();
+      expect(jsonSpy).not.toHaveBeenCalled();
+    });
+
+    it('should pass through SyntaxError without status 400', () => {
+      const err = new SyntaxError('Some syntax error') as SyntaxError & { status?: number };
+      err.status = 500;
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(statusSpy).not.toHaveBeenCalled();
+    });
+
+    it('should pass through SyntaxError without body property', () => {
+      const err = new SyntaxError('Some syntax error') as SyntaxError & { status?: number };
+      err.status = 400;
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(statusSpy).not.toHaveBeenCalled();
+    });
+
+    it('should pass through TypeError', () => {
+      const err = new TypeError('Type error');
+
+      handleJsonParseError(err, req as Request, res as Response, next);
+
+      expect(next).toHaveBeenCalledWith(err);
+      expect(statusSpy).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('security verification', () => {
+    it('should return generic error message for all JSON parse errors', () => {
+      const testCases = [
+        'Unexpected token < in JSON',
+        'Unexpected end of JSON input',
+        'Invalid or unexpected token',
+        '<script>alert(1)</script>',
+        '"><img src=x onerror=alert(1)>',
+      ];
+
+      testCases.forEach((errorMsg) => {
+        const err = new SyntaxError(errorMsg) as SyntaxError & {
+          status?: number;
+          body?: unknown;
+        };
+        err.status = 400;
+        err.body = errorMsg;
+
+        jsonSpy.mockClear();
+        statusSpy.mockClear();
+        (next as jest.Mock).mockClear();
+
+        handleJsonParseError(err, req as Request, res as Response, next);
+
+        const errorResponse = jsonSpy.mock.calls[0][0];
+        // Verify the generic message is always returned, not the user input
+        expect(errorResponse.message).toBe('The request body contains malformed JSON');
+        expect(errorResponse.error).toBe('Invalid JSON format');
+      });
+    });
+  });
+});

packages/api/src/middleware/json.ts

@@ -0,0 +1,40 @@
+import { logger } from '@librechat/data-schemas';
+import type { Request, Response, NextFunction } from 'express';
+
+/**
+ * Middleware to handle JSON parsing errors from express.json()
+ * Prevents user input from being reflected in error messages (XSS prevention)
+ *
+ * This middleware should be placed immediately after express.json() middleware.
+ *
+ * @param err - Error object from express.json()
+ * @param req - Express request object
+ * @param res - Express response object
+ * @param next - Express next function
+ *
+ * @example
+ * app.use(express.json({ limit: '3mb' }));
+ * app.use(handleJsonParseError);
+ */
+export function handleJsonParseError(
+  err: Error & { status?: number; body?: unknown },
+  req: Request,
+  res: Response,
+  next: NextFunction,
+): void {
+  if (err instanceof SyntaxError && err.status === 400 && 'body' in err) {
+    logger.warn('[JSON Parse Error] Invalid JSON received', {
+      path: req.path,
+      method: req.method,
+      ip: req.ip,
+    });
+
+    res.status(400).json({
+      error: 'Invalid JSON format',
+      message: 'The request body contains malformed JSON',
+    });
+    return;
+  }
+
+  next(err);
+}

packages/api/src/prompts/index.ts

@@ -1,2 +1,3 @@
 export * from './format';
 export * from './migration';
+export * from './schemas';

packages/api/src/prompts/schemas.spec.ts

@@ -0,0 +1,222 @@
+import {
+  updatePromptGroupSchema,
+  validatePromptGroupUpdate,
+  safeValidatePromptGroupUpdate,
+} from './schemas';
+
+describe('updatePromptGroupSchema', () => {
+  describe('allowed fields', () => {
+    it('should accept valid name field', () => {
+      const result = updatePromptGroupSchema.safeParse({ name: 'Test Group' });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.name).toBe('Test Group');
+      }
+    });
+
+    it('should accept valid oneliner field', () => {
+      const result = updatePromptGroupSchema.safeParse({ oneliner: 'A short description' });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.oneliner).toBe('A short description');
+      }
+    });
+
+    it('should accept valid category field', () => {
+      const result = updatePromptGroupSchema.safeParse({ category: 'testing' });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.category).toBe('testing');
+      }
+    });
+
+    it('should accept valid projectIds array', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        projectIds: ['proj1', 'proj2'],
+      });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.projectIds).toEqual(['proj1', 'proj2']);
+      }
+    });
+
+    it('should accept valid removeProjectIds array', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        removeProjectIds: ['proj1'],
+      });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.removeProjectIds).toEqual(['proj1']);
+      }
+    });
+
+    it('should accept valid command field', () => {
+      const result = updatePromptGroupSchema.safeParse({ command: 'my-command-123' });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.command).toBe('my-command-123');
+      }
+    });
+
+    it('should accept null command field', () => {
+      const result = updatePromptGroupSchema.safeParse({ command: null });
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data.command).toBeNull();
+      }
+    });
+
+    it('should accept multiple valid fields', () => {
+      const input = {
+        name: 'Updated Name',
+        category: 'new-category',
+        oneliner: 'New description',
+      };
+      const result = updatePromptGroupSchema.safeParse(input);
+      expect(result.success).toBe(true);
+      if (result.success) {
+        expect(result.data).toEqual(input);
+      }
+    });
+
+    it('should accept empty object', () => {
+      const result = updatePromptGroupSchema.safeParse({});
+      expect(result.success).toBe(true);
+    });
+  });
+
+  describe('security - strips sensitive fields', () => {
+    it('should reject author field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        author: '507f1f77bcf86cd799439011',
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject authorName field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        authorName: 'Malicious Author',
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject _id field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        _id: '507f1f77bcf86cd799439011',
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject productionId field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        productionId: '507f1f77bcf86cd799439011',
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject createdAt field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        createdAt: new Date().toISOString(),
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject updatedAt field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        updatedAt: new Date().toISOString(),
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject __v field', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Test',
+        __v: 999,
+      });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject multiple sensitive fields in a single request', () => {
+      const result = updatePromptGroupSchema.safeParse({
+        name: 'Legit Name',
+        author: '507f1f77bcf86cd799439011',
+        authorName: 'Hacker',
+        _id: 'newid123',
+        productionId: 'prodid456',
+        createdAt: '2020-01-01T00:00:00.000Z',
+        __v: 999,
+      });
+      expect(result.success).toBe(false);
+    });
+  });
+
+  describe('validation rules', () => {
+    it('should reject empty name', () => {
+      const result = updatePromptGroupSchema.safeParse({ name: '' });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject name exceeding max length', () => {
+      const result = updatePromptGroupSchema.safeParse({ name: 'a'.repeat(256) });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject oneliner exceeding max length', () => {
+      const result = updatePromptGroupSchema.safeParse({ oneliner: 'a'.repeat(501) });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject category exceeding max length', () => {
+      const result = updatePromptGroupSchema.safeParse({ category: 'a'.repeat(101) });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject command with invalid characters (uppercase)', () => {
+      const result = updatePromptGroupSchema.safeParse({ command: 'MyCommand' });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject command with invalid characters (spaces)', () => {
+      const result = updatePromptGroupSchema.safeParse({ command: 'my command' });
+      expect(result.success).toBe(false);
+    });
+
+    it('should reject command with invalid characters (special)', () => {
+      const result = updatePromptGroupSchema.safeParse({ command: 'my_command!' });
+      expect(result.success).toBe(false);
+    });
+  });
+});
+
+describe('validatePromptGroupUpdate', () => {
+  it('should return validated data for valid input', () => {
+    const input = { name: 'Test', category: 'testing' };
+    const result = validatePromptGroupUpdate(input);
+    expect(result).toEqual(input);
+  });
+
+  it('should throw ZodError for invalid input', () => {
+    expect(() => validatePromptGroupUpdate({ author: 'malicious-id' })).toThrow();
+  });
+});
+
+describe('safeValidatePromptGroupUpdate', () => {
+  it('should return success true for valid input', () => {
+    const result = safeValidatePromptGroupUpdate({ name: 'Test' });
+    expect(result.success).toBe(true);
+  });
+
+  it('should return success false for invalid input with errors', () => {
+    const result = safeValidatePromptGroupUpdate({ author: 'malicious-id' });
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error.errors.length).toBeGreaterThan(0);
+    }
+  });
+});

packages/api/src/prompts/schemas.ts

@@ -0,0 +1,53 @@
+import { z } from 'zod';
+import { Constants } from 'librechat-data-provider';
+
+/**
+ * Schema for validating prompt group update payloads.
+ * Only allows fields that users should be able to modify.
+ * Sensitive fields like author, authorName, _id, productionId, etc. are excluded.
+ */
+export const updatePromptGroupSchema = z
+  .object({
+    /** The name of the prompt group */
+    name: z.string().min(1).max(255).optional(),
+    /** Short description/oneliner for the prompt group */
+    oneliner: z.string().max(500).optional(),
+    /** Category for organizing prompt groups */
+    category: z.string().max(100).optional(),
+    /** Project IDs to add for sharing */
+    projectIds: z.array(z.string()).optional(),
+    /** Project IDs to remove from sharing */
+    removeProjectIds: z.array(z.string()).optional(),
+    /** Command shortcut for the prompt group */
+    command: z
+      .string()
+      .max(Constants.COMMANDS_MAX_LENGTH as number)
+      .regex(/^[a-z0-9-]*$/, {
+        message: 'Command must only contain lowercase alphanumeric characters and hyphens',
+      })
+      .optional()
+      .nullable(),
+  })
+  .strict();
+
+export type TUpdatePromptGroupSchema = z.infer<typeof updatePromptGroupSchema>;
+
+/**
+ * Validates and sanitizes a prompt group update payload.
+ * Returns only the allowed fields, stripping any sensitive fields.
+ * @param data - The raw request body to validate
+ * @returns The validated and sanitized payload
+ * @throws ZodError if validation fails
+ */
+export function validatePromptGroupUpdate(data: unknown): TUpdatePromptGroupSchema {
+  return updatePromptGroupSchema.parse(data);
+}
+
+/**
+ * Safely validates a prompt group update payload without throwing.
+ * @param data - The raw request body to validate
+ * @returns A SafeParseResult with either the validated data or validation errors
+ */
+export function safeValidatePromptGroupUpdate(data: unknown) {
+  return updatePromptGroupSchema.safeParse(data);
+}