Merge pull request #10259 from weseek/fix/170423-codeql-problems

fix: CodeQL problems

apps/app/src/server/routes/apiv3/attachment.js

@@ -1,3 +1,4 @@
+import { SCOPE } from '@growi/core/dist/interfaces';
 import { ErrorV3 } from '@growi/core/dist/models';
 import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
 import express from 'express';
@@ -5,7 +6,6 @@ import multer from 'multer';
 import autoReap from 'multer-autoreap';
 
 import { SupportedAction } from '~/interfaces/activity';
-import { SCOPE } from '@growi/core/dist/interfaces';
 import { AttachmentType } from '~/server/interfaces/attachment';
 import { accessTokenParser } from '~/server/middlewares/access-token-parser';
 import { Attachment } from '~/server/models/attachment';
@@ -158,7 +158,7 @@ module.exports = (crowi) => {
       query('fileSize').isNumeric().exists({ checkNull: true }).withMessage('fileSize is required'),
     ],
     retrieveAddAttachment: [
-      body('page_id').isString().exists({ checkNull: true }).withMessage('page_id is required'),
+      body('page_id').isMongoId().exists({ checkNull: true }).withMessage('page_id is required'),
     ],
   };
 

apps/app/src/server/routes/apiv3/page/update-page.ts

@@ -2,6 +2,7 @@ import { Origin, allOrigin, getIdForRef } from '@growi/core';
 import type {
   IPage, IRevisionHasId, IUserHasId,
 } from '@growi/core';
+import { SCOPE } from '@growi/core/dist/interfaces';
 import { ErrorV3 } from '@growi/core/dist/models';
 import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
 import { isTopPage, isUsersProtectedPages } from '@growi/core/dist/utils/page-path-utils';
@@ -15,7 +16,6 @@ import { isAiEnabled } from '~/features/openai/server/services';
 import { SupportedAction, SupportedTargetModel } from '~/interfaces/activity';
 import { type IApiv3PageUpdateParams, PageUpdateErrorCode } from '~/interfaces/apiv3';
 import type { IOptionsForUpdate } from '~/interfaces/page';
-import { SCOPE } from '@growi/core/dist/interfaces';
 import type Crowi from '~/server/crowi';
 import { accessTokenParser } from '~/server/middlewares/access-token-parser';
 import { generateAddActivityMiddleware } from '~/server/middlewares/add-activity';
@@ -52,7 +52,8 @@ export const updatePageHandlersFactory: UpdatePageHandlersFactory = (crowi) => {
 
   // define validators for req.body
   const validator: ValidationChain[] = [
-    body('pageId').exists().not().isEmpty({ ignore_whitespace: true })
+    body('pageId').isMongoId().exists().not()
+      .isEmpty({ ignore_whitespace: true })
       .withMessage("'pageId' must be specified"),
     body('revisionId').optional().exists().not()
       .isEmpty({ ignore_whitespace: true })

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

@@ -4,7 +4,7 @@ import createError from 'http-errors';
 import { mock } from 'vitest-mock-extended';
 
 import type { LsxApiParams, LsxApiResponseData } from '../../../interfaces/api';
-import { listPages } from '.';
+import { addFilterCondition, listPages } from '.';
 import type { PageQuery, PageQueryBuilder } from './generate-base-query';
 
 interface IListPagesRequest
@@ -158,4 +158,88 @@ describe('listPages', () => {
       expect(resStatusMock.send).toHaveBeenCalledWith('error for test');
     });
   });
+
+  describe('addFilterCondition', () => {
+    const queryMock = mock<PageQuery>();
+    // and method returns mock itself
+    queryMock.and.mockReturnValue(queryMock);
+
+    beforeEach(() => {
+      queryMock.and.mockClear();
+    });
+
+    it('should call query.and with the correct regex when filter starts with "^"', () => {
+      // setup
+      const pagePath = '/parent';
+      const optionsFilter = '^child';
+      const expectedRegex = /^\/parent\/child/;
+
+      // when
+      addFilterCondition(queryMock, pagePath, optionsFilter);
+
+      // then
+      expect(queryMock.and).toHaveBeenCalledWith({ path: expectedRegex });
+    });
+
+    it('should call query.and with the correct regex when filter does not start with "^"', () => {
+      // setup
+      const pagePath = '/parent';
+      const optionsFilter = 'child';
+      const expectedRegex = /^\/parent\/.*child/;
+
+      // when
+      addFilterCondition(queryMock, pagePath, optionsFilter);
+
+      // then
+      expect(queryMock.and).toHaveBeenCalledWith({ path: expectedRegex });
+    });
+
+    it('should properly escape regex meta-characters like "[" in filter', () => {
+      // setup
+      const pagePath = '/parent';
+      const optionsFilter = '['; // Invalid regex
+      const expectedRex = /^\/parent\/.*\[/;
+
+      // when
+      addFilterCondition(queryMock, pagePath, optionsFilter);
+
+      // then
+      expect(queryMock.and).toHaveBeenCalledWith({ path: expectedRex });
+    });
+
+    it('should call query.and with "$not" when isExceptFilter is true', () => {
+      // setup
+      const pagePath = '/parent';
+      const optionsFilter = 'child';
+      const expectedRegex = /^\/parent\/.*child/;
+
+      // when
+      addFilterCondition(queryMock, pagePath, optionsFilter, true);
+
+      // then
+      expect(queryMock.and).toHaveBeenCalledWith({
+        path: { $not: expectedRegex },
+      });
+    });
+
+    it('should throw an error when optionsFilter is null', () => {
+      // setup
+      const pagePath = '/parent';
+
+      // when & then
+      expect(() => addFilterCondition(queryMock, pagePath, null)).toThrow(
+        createError(400, 'filter option require value in regular expression.'),
+      );
+    });
+
+    it('should throw an error when optionsFilter is true', () => {
+      // setup
+      const pagePath = '/parent';
+
+      // when & then
+      expect(() => addFilterCondition(queryMock, pagePath, true)).toThrow(
+        createError(400, 'filter option require value in regular expression.'),
+      );
+    });
+  });
 });

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

@@ -18,7 +18,7 @@ const { addTrailingSlash, removeTrailingSlash } = pathUtils;
 /**
  * add filter condition that filter fetched pages
  */
-function addFilterCondition(
+export function addFilterCondition(
   query,
   pagePath,
   optionsFilter,
@@ -38,11 +38,11 @@ function addFilterCondition(
   try {
     if (optionsFilter.charAt(0) === '^') {
       // move '^' to the first of path
-      filterPath = new RegExp(
-        `^${pagePathForRegexp}${optionsFilter.slice(1, optionsFilter.length)}`,
-      );
+      const escapedFilter = escapeStringRegexp(optionsFilter.slice(1));
+      filterPath = new RegExp(`^${pagePathForRegexp}${escapedFilter}`);
     } else {
-      filterPath = new RegExp(`^${pagePathForRegexp}.*${optionsFilter}`);
+      const escapedFilter = escapeStringRegexp(optionsFilter);
+      filterPath = new RegExp(`^${pagePathForRegexp}.*${escapedFilter}`);
     }
   } catch (err) {
     throw createError(400, err);