From 98def949ecd7f01585f8809468707c0cbaba80a9 Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Tue, 5 May 2026 08:26:38 -0400
Subject: [PATCH] Merge pull request #33040 from overleaf/em-parse-req-errors

Replace isZodErrorLike with custom error types in request validation

GitOrigin-RevId: 9cb453a2cde595a00f5049e4829ea9e3dbe17b28
---
 libraries/validation-tools/Errors.js          | 28 +++++++++++++++++--
 .../validation-tools/handleValidationError.js | 25 ++++++++++-------
 libraries/validation-tools/index.js           |  5 ++--
 libraries/validation-tools/parseReq.js        |  6 ++--
 .../test/unit/src/validateReq.test.ts         | 10 ++++---
 server-ce/test/host-admin.js                  | 17 +++++++----
 services/notifications/app.ts                 | 10 ++++---
 services/real-time/app/js/Router.js           |  5 +++-
 .../src/Features/Errors/ErrorController.mjs   | 20 ++++++++-----
 .../Features/Security/LoginRateLimiter.mjs    |  7 ++++-
 .../web/app/src/infrastructure/Validation.mjs |  2 +-
 .../acceptance/src/AuthenticationTests.mjs    | 23 ++-------------
 .../test/acceptance/src/ProjectCRUDTests.mjs  |  3 +-
 .../acceptance/src/ProjectInviteTests.mjs     | 13 ---------
 .../SubscriptionController.test.mjs           |  2 +-
 .../unit/src/Tags/TagsController.test.mjs     |  4 +--
 16 files changed, 100 insertions(+), 80 deletions(-)

diff --git a/libraries/validation-tools/Errors.js b/libraries/validation-tools/Errors.js
index e6a12d4e54..197289801f 100644
--- a/libraries/validation-tools/Errors.js
+++ b/libraries/validation-tools/Errors.js
@@ -1,5 +1,29 @@
+// @ts-check
+
 const OError = require('@overleaf/o-error')
 
-class ParamsError extends OError {}
+/**
+ * @typedef {import('zod').ZodError} ZodError
+ */
 
-module.exports = { ParamsError }
+class InvalidRequestError extends OError {
+  /**
+   * @param {ZodError} zodError
+   */
+  constructor(zodError) {
+    super('Invalid request', {}, zodError)
+    this.zodError = zodError
+  }
+}
+
+class InvalidParamsError extends OError {
+  /**
+   * @param {ZodError} zodError
+   */
+  constructor(zodError) {
+    super('Invalid request parameters', {}, zodError)
+    this.zodError = zodError
+  }
+}
+
+module.exports = { InvalidParamsError, InvalidRequestError }
diff --git a/libraries/validation-tools/handleValidationError.js b/libraries/validation-tools/handleValidationError.js
index a5038448eb..f7b24528e5 100644
--- a/libraries/validation-tools/handleValidationError.js
+++ b/libraries/validation-tools/handleValidationError.js
@@ -1,15 +1,20 @@
-const { isZodErrorLike, fromError } = require('zod-validation-error')
+const { fromError } = require('zod-validation-error')
+const { InvalidParamsError, InvalidRequestError } = require('./Errors')
 
 function createHandleValidationError(statusCode = 400) {
-  return [
-    (err, req, res, next) => {
-      if (!isZodErrorLike(err)) {
-        return next(err)
-      }
-
-      res.status(statusCode).json({ ...fromError(err), statusCode })
-    },
-  ]
+  return (err, req, res, next) => {
+    if (err instanceof InvalidParamsError) {
+      res
+        .status(404)
+        .json({ error: fromError(err.zodError).toString(), statusCode: 404 })
+    } else if (err instanceof InvalidRequestError) {
+      res
+        .status(statusCode)
+        .json({ error: fromError(err.zodError).toString(), statusCode })
+    } else {
+      next(err)
+    }
+  }
 }
 
 const handleValidationError = createHandleValidationError(400)
diff --git a/libraries/validation-tools/index.js b/libraries/validation-tools/index.js
index c456a3a7bf..6a4169117f 100644
--- a/libraries/validation-tools/index.js
+++ b/libraries/validation-tools/index.js
@@ -1,4 +1,4 @@
-const { ParamsError } = require('./Errors')
+const { InvalidParamsError, InvalidRequestError } = require('./Errors')
 const { z } = require('zod')
 const { zz } = require('./zodHelpers')
 const { parseReq } = require('./parseReq')
@@ -15,5 +15,6 @@ module.exports = {
   parseReq,
   handleValidationError,
   createHandleValidationError,
-  ParamsError,
+  InvalidRequestError,
+  InvalidParamsError,
 }
diff --git a/libraries/validation-tools/parseReq.js b/libraries/validation-tools/parseReq.js
index cb3c87c659..28818a4eb8 100644
--- a/libraries/validation-tools/parseReq.js
+++ b/libraries/validation-tools/parseReq.js
@@ -1,5 +1,5 @@
 // @ts-check
-const { ParamsError } = require('./Errors')
+const { InvalidRequestError, InvalidParamsError } = require('./Errors')
 
 /**
  * @typedef {import('zod').ZodType} ZodType
@@ -25,9 +25,9 @@ function parseReq(req, schema) {
     return parsed.data
   } else if (parsed.error.issues.some(issue => issue.path[0] === 'params')) {
     // Parts of the URL path failed to validate; throw a specific error
-    throw new ParamsError('Invalid params').withCause(parsed.error)
+    throw new InvalidParamsError(parsed.error)
   } else {
-    throw parsed.error
+    throw new InvalidRequestError(parsed.error)
   }
 }
 
diff --git a/libraries/validation-tools/test/unit/src/validateReq.test.ts b/libraries/validation-tools/test/unit/src/validateReq.test.ts
index a30e14c299..69bb516458 100644
--- a/libraries/validation-tools/test/unit/src/validateReq.test.ts
+++ b/libraries/validation-tools/test/unit/src/validateReq.test.ts
@@ -54,7 +54,7 @@ describe('parseReq', () => {
             }),
           })
         )
-      ).toThrowError(expect.objectContaining({ name: 'ParamsError' }))
+      ).toThrowError(expect.objectContaining({ name: 'InvalidParamsError' }))
     })
 
     it('should throw an error containing issues if the schema is invalid', () => {
@@ -75,9 +75,11 @@ describe('parseReq', () => {
         )
       ).toThrowError(
         expect.objectContaining({
-          issues: expect.arrayContaining([
-            expect.objectContaining({ path: ['body', 'name'] }),
-          ]),
+          zodError: expect.objectContaining({
+            issues: expect.arrayContaining([
+              expect.objectContaining({ path: ['body', 'name'] }),
+            ]),
+          }),
         })
       )
     })
diff --git a/server-ce/test/host-admin.js b/server-ce/test/host-admin.js
index c871c6ed23..a56cb218a1 100644
--- a/server-ce/test/host-admin.js
+++ b/server-ce/test/host-admin.js
@@ -5,8 +5,12 @@ import { execFile as execFileCb } from 'node:child_process'
 import bodyParser from 'body-parser'
 import express from 'express'
 import YAML from 'js-yaml'
-import { isZodErrorLike } from 'zod-validation-error'
-import { ParamsError, parseReq, z } from '@overleaf/validation-tools'
+import {
+  InvalidParamsError,
+  InvalidRequestError,
+  parseReq,
+  z,
+} from '@overleaf/validation-tools'
 import { expressify } from '@overleaf/promise-utils'
 
 const execFile = promisify(execFileCb)
@@ -474,12 +478,13 @@ app.delete(
 )
 
 app.use((error, req, res, next) => {
-  if (error instanceof ParamsError) {
+  if (error instanceof InvalidParamsError) {
     res.status(404).json({ error })
-  } else if (isZodErrorLike(error)) {
-    res.status(400).json({ error })
+  } else if (error instanceof InvalidRequestError) {
+    res.status(400).json({ error: error.zodError })
+  } else {
+    next(error)
   }
-  next(error)
 })
 
 purgeDataDir()
diff --git a/services/notifications/app.ts b/services/notifications/app.ts
index 71e5a12017..3232e24be1 100644
--- a/services/notifications/app.ts
+++ b/services/notifications/app.ts
@@ -13,8 +13,10 @@ import methodOverride from 'method-override'
 import { mongoClient } from './app/js/mongodb.js'
 import NotificationsController from './app/js/NotificationsController.ts'
 import HealthCheckController from './app/js/HealthCheckController.ts'
-import { isZodErrorLike } from 'zod-validation-error'
-import { ParamsError } from '@overleaf/validation-tools'
+import {
+  InvalidParamsError,
+  InvalidRequestError,
+} from '@overleaf/validation-tools'
 
 const app = express()
 
@@ -56,10 +58,10 @@ const handleApiError: ErrorRequestHandler = (
   next: NextFunction
 ) => {
   req.logger.addFields({ err })
-  if (err instanceof ParamsError) {
+  if (err instanceof InvalidParamsError) {
     req.logger.setLevel('warn')
     res.sendStatus(404)
-  } else if (isZodErrorLike(err)) {
+  } else if (err instanceof InvalidRequestError) {
     req.logger.setLevel('warn')
     res.sendStatus(400)
   } else {
diff --git a/services/real-time/app/js/Router.js b/services/real-time/app/js/Router.js
index e29873fcbf..794939da4e 100644
--- a/services/real-time/app/js/Router.js
+++ b/services/real-time/app/js/Router.js
@@ -40,7 +40,7 @@ export default Router = {
     attrs.client_id = client.id
     attrs.err = error
     attrs.method = method
-    if (isZodErrorLike(error)) {
+    if (attrs.validation && isZodErrorLike(error)) {
       logger.info(attrs, 'validation error')
       let message = 'invalid'
       try {
@@ -456,6 +456,7 @@ export default Router = {
           joinDocSchema.parse({ doc_id: docId, fromVersion, options })
         } catch (error) {
           return Router._handleError(callback, error, client, 'joinDoc', {
+            validation: 1,
             disconnect: 1,
           })
         }
@@ -485,6 +486,7 @@ export default Router = {
           zz.objectId().parse(docId)
         } catch (error) {
           return Router._handleError(callback, error, client, 'joinDoc', {
+            validation: 1,
             disconnect: 1,
           })
         }
@@ -570,6 +572,7 @@ export default Router = {
           applyOtUpdateSchema.parse({ doc_id: docId, update })
         } catch (error) {
           return Router._handleError(callback, error, client, 'applyOtUpdate', {
+            validation: 1,
             disconnect: 1,
           })
         }
diff --git a/services/web/app/src/Features/Errors/ErrorController.mjs b/services/web/app/src/Features/Errors/ErrorController.mjs
index b70774b2a6..37316304a8 100644
--- a/services/web/app/src/Features/Errors/ErrorController.mjs
+++ b/services/web/app/src/Features/Errors/ErrorController.mjs
@@ -1,11 +1,14 @@
-import { isZodErrorLike, fromZodError } from 'zod-validation-error'
+import { fromZodError } from 'zod-validation-error'
+import {
+  InvalidRequestError,
+  InvalidParamsError,
+} from '@overleaf/validation-tools'
 import Errors, { NotFoundError } from './Errors.js'
 import SessionManager from '../Authentication/SessionManager.mjs'
 import SamlLogHandler from '../SamlLog/SamlLogHandler.mjs'
 import HttpErrorHandler from './HttpErrorHandler.mjs'
 import { plainTextResponse } from '../../infrastructure/Response.mjs'
 import { expressifyErrorHandler } from '@overleaf/promise-utils'
-import { ParamsError } from '@overleaf/validation-tools'
 
 function notFound(req, res) {
   res.status(404)
@@ -42,7 +45,7 @@ async function handleError(error, req, res, next) {
     if (shouldSendErrorResponse) {
       notFound(req, res)
     }
-  } else if (error instanceof ParamsError) {
+  } else if (error instanceof InvalidParamsError) {
     req.logger.setLevel('warn')
     if (shouldSendErrorResponse) {
       notFound(req, res)
@@ -104,11 +107,11 @@ async function handleError(error, req, res, next) {
       res.status(400)
       plainTextResponse(res, error.message)
     }
-  } else if (isZodErrorLike(error)) {
+  } else if (error instanceof InvalidRequestError) {
     req.logger.setLevel('warn')
     res.status(400)
     if (shouldSendErrorResponse) {
-      const validationError = fromZodError(error)
+      const validationError = fromZodError(error.zodError)
       res.render('general/400', { message: validationError.message })
     }
   } else {
@@ -126,7 +129,10 @@ async function handleError(error, req, res, next) {
 
 function handleApiError(err, req, res, next) {
   req.logger.addFields({ err })
-  if (err instanceof Errors.NotFoundError || err instanceof ParamsError) {
+  if (
+    err instanceof Errors.NotFoundError ||
+    err instanceof InvalidParamsError
+  ) {
     req.logger.setLevel('warn')
     res.sendStatus(404)
   } else if (
@@ -141,7 +147,7 @@ function handleApiError(err, req, res, next) {
   } else if (err instanceof Errors.ForbiddenError) {
     req.logger.setLevel('warn')
     res.sendStatus(403)
-  } else if (isZodErrorLike(err)) {
+  } else if (err instanceof InvalidRequestError) {
     req.logger.setLevel('warn')
     res.sendStatus(400)
   } else {
diff --git a/services/web/app/src/Features/Security/LoginRateLimiter.mjs b/services/web/app/src/Features/Security/LoginRateLimiter.mjs
index 4fb032f222..276904123f 100644
--- a/services/web/app/src/Features/Security/LoginRateLimiter.mjs
+++ b/services/web/app/src/Features/Security/LoginRateLimiter.mjs
@@ -2,6 +2,7 @@ import { RateLimiter } from '../../infrastructure/RateLimiter.mjs'
 import { callbackify } from '@overleaf/promise-utils'
 import Settings from '@overleaf/settings'
 import EmailHelper from '../Helpers/EmailHelper.mjs'
+import { InvalidRequestError } from '@overleaf/validation-tools'
 
 const rateLimiterLoginEmail = new RateLimiter(
   'login',
@@ -12,7 +13,11 @@ const rateLimiterLoginEmail = new RateLimiter(
 )
 
 async function processLoginRequest(email) {
-  email = EmailHelper.emailSchema.parse(email)
+  try {
+    email = EmailHelper.emailSchema.parse(email)
+  } catch (err) {
+    throw new InvalidRequestError(err)
+  }
   try {
     await rateLimiterLoginEmail.consume(email, 1, {
       method: 'email',
diff --git a/services/web/app/src/infrastructure/Validation.mjs b/services/web/app/src/infrastructure/Validation.mjs
index 41349520bb..76a8a7cc39 100644
--- a/services/web/app/src/infrastructure/Validation.mjs
+++ b/services/web/app/src/infrastructure/Validation.mjs
@@ -2,7 +2,7 @@
 
 import { parseReq, z, zz } from '@overleaf/validation-tools'
 
-export { ParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
+export { InvalidParamsError, parseReq, z, zz } from '@overleaf/validation-tools'
 
 export default {
   parseReq,
diff --git a/services/web/test/acceptance/src/AuthenticationTests.mjs b/services/web/test/acceptance/src/AuthenticationTests.mjs
index d3acb19de3..16f8c1a704 100644
--- a/services/web/test/acceptance/src/AuthenticationTests.mjs
+++ b/services/web/test/acceptance/src/AuthenticationTests.mjs
@@ -176,31 +176,12 @@ describe('Authentication', function () {
       }
     })
     it('should return 400 with bad email (missing @)', async function () {
-      const { statusCode, body } = await tryLoginWithEmail('foo')
+      const { statusCode } = await tryLoginWithEmail('foo')
       expect(statusCode).to.equal(400)
-      expect(body).to.deep.equal({
-        name: 'ZodValidationError',
-        details: [
-          { code: 'custom', path: [], message: 'Invalid email address' },
-        ],
-        statusCode: 400,
-      })
     })
     it('should return 400 with bad email (number)', async function () {
-      const { statusCode, body } = await tryLoginWithEmail(1)
+      const { statusCode } = await tryLoginWithEmail(1)
       expect(statusCode).to.equal(400)
-      expect(body).to.deep.equal({
-        name: 'ZodValidationError',
-        details: [
-          {
-            expected: 'string',
-            code: 'invalid_type',
-            path: [],
-            message: 'Invalid input: expected string, received number',
-          },
-        ],
-        statusCode: 400,
-      })
     })
   })
 })
diff --git a/services/web/test/acceptance/src/ProjectCRUDTests.mjs b/services/web/test/acceptance/src/ProjectCRUDTests.mjs
index af711f29be..0a95d254b3 100644
--- a/services/web/test/acceptance/src/ProjectCRUDTests.mjs
+++ b/services/web/test/acceptance/src/ProjectCRUDTests.mjs
@@ -219,14 +219,13 @@ describe('Project CRUD', function () {
     })
     it('returns a 400 when publicAccessLevel is an unsupported access level', async function () {
       await this.user.makePrivate(this.projectId)
-      const { response, body } = await this.user.doRequest('POST', {
+      const { response } = await this.user.doRequest('POST', {
         url: `/project/${this.projectId}/settings/admin`,
         json: {
           publicAccessLevel: 'readOnly',
         },
       })
       expect(response.statusCode).to.equal(400)
-      expect(body.details[0].message).to.equal('unexpected access level')
       const project = await Project.findById(this.projectId).exec()
       expect(project.publicAccesLevel).to.equal('private')
     })
diff --git a/services/web/test/acceptance/src/ProjectInviteTests.mjs b/services/web/test/acceptance/src/ProjectInviteTests.mjs
index 525a1bbc82..926d9dcfdc 100644
--- a/services/web/test/acceptance/src/ProjectInviteTests.mjs
+++ b/services/web/test/acceptance/src/ProjectInviteTests.mjs
@@ -375,11 +375,6 @@ describe('ProjectInviteTests', function () {
                 return done(err)
               }
               expect(response.statusCode).to.equal(400)
-              expect(body.details).to.have.lengthOf(1)
-              expect(response.body.details[0].path).to.eql(['body', 'email'])
-              expect(response.body.details[0].message).to.equal(
-                'Invalid input: expected string, received object'
-              )
               done()
             }
           )
@@ -404,14 +399,6 @@ describe('ProjectInviteTests', function () {
                 return done(err)
               }
               expect(response.statusCode).to.equal(400)
-              expect(body.details).to.have.lengthOf(1)
-              expect(response.body.details[0].path).to.eql([
-                'body',
-                'privileges',
-              ])
-              expect(response.body.details[0].message).to.equal(
-                'Invalid option: expected one of "readOnly"|"readAndWrite"|"review"'
-              )
               done()
             }
           )
diff --git a/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs
index 3902d5c8d4..fd2a77e2dc 100644
--- a/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs
+++ b/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs
@@ -685,7 +685,7 @@ describe('SubscriptionController', function () {
       ctx.next = sinon.stub()
       await expect(
         ctx.SubscriptionController.pauseSubscription(ctx.req, ctx.res, ctx.next)
-      ).to.be.rejectedWith('Invalid params')
+      ).to.be.rejectedWith('Invalid request parameters')
     })
 
     it('should throw an error if an invalid pause length is provided', async function (ctx) {
diff --git a/services/web/test/unit/src/Tags/TagsController.test.mjs b/services/web/test/unit/src/Tags/TagsController.test.mjs
index 9a8c249f9f..b04457cd0f 100644
--- a/services/web/test/unit/src/Tags/TagsController.test.mjs
+++ b/services/web/test/unit/src/Tags/TagsController.test.mjs
@@ -1,6 +1,6 @@
 import { assert, beforeEach, describe, it, vi } from 'vitest'
 import sinon from 'sinon'
-import { ZodError } from 'zod'
+import { InvalidRequestError } from '@overleaf/validation-tools'
 
 const modulePath = '../../../../app/src/Features/Tags/TagsController.mjs'
 
@@ -202,7 +202,7 @@ describe('TagsController', function () {
     it('without a name', function (ctx) {
       ctx.req.body = { name: undefined }
       ctx.TagsController.renameTag(ctx.req, ctx.res).should.be.rejectedWith(
-        ZodError
+        InvalidRequestError
       )
     })
   })