diff --git a/packages/remark-lsx/package.json b/packages/remark-lsx/package.json index 4abf627a948..4ed5e8c8c0f 100644 --- a/packages/remark-lsx/package.json +++ b/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", diff --git a/packages/remark-lsx/src/server/index.ts b/packages/remark-lsx/src/server/index.ts index 6efaa1c4285..04ea109bfd8 100644 --- a/packages/remark-lsx/src/server/index.ts +++ b/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; diff --git a/packages/remark-lsx/src/server/routes/list-pages/index.spec.ts b/packages/remark-lsx/src/server/routes/list-pages/index.spec.ts index 56987fb1c3f..e7da15f9c41 100644 --- a/packages/remark-lsx/src/server/routes/list-pages/index.spec.ts +++ b/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 { + 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(); + const reqMock = mock(); const resMock = mock(); const resStatusMock = mock(); resMock.status.calledWith(400).mockReturnValue(resStatusMock); @@ -46,7 +49,7 @@ describe('listPages', () => { describe('with num option', () => { - const reqMock = mock(); + const reqMock = mock(); reqMock.query = { pagePath: '/Sandbox' }; const builderMock = mock(); @@ -97,7 +100,7 @@ describe('listPages', () => { it('returns 500 HTTP response when an unexpected error occured', async() => { // setup - const reqMock = mock(); + const reqMock = mock(); 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(); + const reqMock = mock(); reqMock.query = { pagePath: '/Sandbox' }; // an http-errors instance will be thrown by addNumConditionMock diff --git a/packages/remark-lsx/src/server/routes/list-pages/index.ts b/packages/remark-lsx/src/server/routes/list-pages/index.ts index 953bd87f1e9..e7eb17d7052 100644 --- a/packages/remark-lsx/src/server/routes/list-pages/index.ts +++ b/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 { + user: IUser, +} + -export const listPages = async(req: Request & { user: IUser }, res: Response): Promise => { +export const listPages = async(req: IListPagesRequest, res: Response): Promise => { 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 {