Merge pull request #9182 from weseek/fix/154290-154296-add-validators-to-lsx-api

fix: Add validators to lsx API

packages/remark-lsx/package.json

@@ -38,9 +38,11 @@
     "@growi/ui": "link:../ui",
     "escape-string-regexp": "^4.0.0",
     "express": "^4.20.0",
+    "express-validator": "^6.14.0",
     "http-errors": "^2.0.0",
     "mongoose": "^6.11.3",
-    "swr": "^2.2.2"
+    "swr": "^2.2.2",
+    "xss": "^1.0.15"
   },
   "devDependencies": {
     "eslint-plugin-regex": "^1.8.0",

packages/remark-lsx/src/server/index.ts

@@ -1,4 +1,8 @@
-import type { Request, Response } from 'express';
+import type { NextFunction, Request, Response } from 'express';
+import { query, validationResult } from 'express-validator';
+import { FilterXSS } from 'xss';
+
+import type { LsxApiOptions } from '../interfaces/api';
 
 import { listPages } from './routes/list-pages';
 
@@ -6,12 +10,45 @@ const loginRequiredFallback = (req: Request, res: Response) => {
   return res.status(403).send('login required');
 };
 
+const filterXSS = new FilterXSS();
+
+const lsxValidator = [
+  query('pagePath').notEmpty().isString(),
+  query('offset').optional().isInt(),
+  query('limit').optional().isInt(),
+  query('options')
+    .optional()
+    .customSanitizer((options) => {
+      try {
+        const jsonData: LsxApiOptions = JSON.parse(options);
+
+        Object.keys(jsonData).forEach((key) => {
+          jsonData[key] = filterXSS.process(jsonData[key]);
+        });
+
+        return jsonData;
+      }
+      catch (err) {
+        throw new Error('Invalid JSON format in options');
+      }
+    }),
+  query('options.*').optional().isString(),
+];
+
+const paramValidator = (req: Request, _: Response, next: NextFunction) => {
+  const errObjArray = validationResult(req);
+  if (errObjArray.isEmpty()) {
+    return next();
+  }
+  return new Error('Invalid lsx parameter');
+};
+
 // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
 const middleware = (crowi: any, app: any): void => {
   const loginRequired = crowi.require('../middlewares/login-required')(crowi, true, loginRequiredFallback);
   const accessTokenParser = crowi.require('../middlewares/access-token-parser')(crowi);
 
-  app.get('/_api/lsx', accessTokenParser, loginRequired, listPages);
+  app.get('/_api/lsx', accessTokenParser, loginRequired, lsxValidator, paramValidator, listPages);
 };
 
 export default middleware;

packages/remark-lsx/src/server/routes/list-pages/index.spec.ts

@@ -3,12 +3,15 @@ import type { Request, Response } from 'express';
 import createError from 'http-errors';
 import { mock } from 'vitest-mock-extended';
 
-import type { LsxApiResponseData } from '../../../interfaces/api';
+import type { LsxApiResponseData, LsxApiParams } from '../../../interfaces/api';
 
 import type { PageQuery, PageQueryBuilder } from './generate-base-query';
 
 import { listPages } from '.';
 
+interface IListPagesRequest extends Request<undefined, undefined, undefined, LsxApiParams> {
+  user: IUser,
+}
 
 // mocking modules
 const mocks = vi.hoisted(() => {
@@ -30,7 +33,7 @@ describe('listPages', () => {
 
   it("returns 400 HTTP response when the query 'pagePath' is undefined", async() => {
     // setup
-    const reqMock = mock<Request & { user: IUser }>();
+    const reqMock = mock<IListPagesRequest>();
     const resMock = mock<Response>();
     const resStatusMock = mock<Response>();
     resMock.status.calledWith(400).mockReturnValue(resStatusMock);
@@ -46,7 +49,7 @@ describe('listPages', () => {
 
   describe('with num option', () => {
 
-    const reqMock = mock<Request & { user: IUser }>();
+    const reqMock = mock<IListPagesRequest>();
     reqMock.query = { pagePath: '/Sandbox' };
 
     const builderMock = mock<PageQueryBuilder>();
@@ -97,7 +100,7 @@ describe('listPages', () => {
 
     it('returns 500 HTTP response when an unexpected error occured', async() => {
       // setup
-      const reqMock = mock<Request & { user: IUser }>();
+      const reqMock = mock<IListPagesRequest>();
       reqMock.query = { pagePath: '/Sandbox' };
 
       // an Error instance will be thrown by addNumConditionMock
@@ -124,7 +127,7 @@ describe('listPages', () => {
 
     it('returns 400 HTTP response when the value is invalid', async() => {
       // setup
-      const reqMock = mock<Request & { user: IUser }>();
+      const reqMock = mock<IListPagesRequest>();
       reqMock.query = { pagePath: '/Sandbox' };
 
       // an http-errors instance will be thrown by addNumConditionMock

packages/remark-lsx/src/server/routes/list-pages/index.ts

@@ -56,20 +56,23 @@ function addExceptCondition(query, pagePath, optionsFilter): PageQuery {
   return addFilterCondition(query, pagePath, optionsFilter, true);
 }
 
+interface IListPagesRequest extends Request<undefined, undefined, undefined, LsxApiParams> {
+  user: IUser,
+}
+
 
-export const listPages = async(req: Request & { user: IUser }, res: Response): Promise<Response> => {
+export const listPages = async(req: IListPagesRequest, res: Response): Promise<Response> => {
   const user = req.user;
 
-  // TODO: use express-validator
   if (req.query.pagePath == null) {
-    return res.status(400).send("The 'pagePath' query must not be null.");
+    return res.status(400).send("the 'pagepath' query must not be null.");
   }
 
   const params: LsxApiParams = {
-    pagePath: removeTrailingSlash(req.query.pagePath.toString()),
-    offset: req.query?.offset != null ? Number(req.query.offset) : undefined,
-    limit: req.query?.limit != null ? Number(req.query?.limit) : undefined,
-    options: req.query?.options != null ? JSON.parse(req.query.options.toString()) : {},
+    pagePath: removeTrailingSlash(req.query.pagePath),
+    offset: req.query?.offset,
+    limit: req.query?.limit,
+    options: req.query?.options ?? {},
   };
 
   const {