Fix/validate site name

src/middleware/__tests__/routeChecker.ts

@@ -0,0 +1,65 @@
+import { NextFunction, Request, Response } from "express"
+
+import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData"
+import { BadRequestError } from "@root/errors/BadRequestError"
+
+import { RouteCheckerMiddleware } from "../routeChecker"
+
+describe("Authorization middleware", () => {
+  const TEST_SITE_NAME = "site-name"
+  const TEST_INVALID_SITE_NAME = "blah%2Fblah"
+  const mockRes = ({} as unknown) as Response
+  const mockNext = jest.fn() as NextFunction
+
+  const routeCheckerMiddleware = new RouteCheckerMiddleware()
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  describe("verifySiteName", () => {
+    it("allows through proper site names", async () => {
+      // Arrange
+      const mockReq = ({
+        params: { siteName: TEST_SITE_NAME },
+      } as unknown) as Request<
+        never,
+        unknown,
+        unknown,
+        never,
+        { userWithSiteSessionData: UserWithSiteSessionData }
+      >
+
+      // Act
+      await routeCheckerMiddleware.verifySiteName(mockReq, mockRes, mockNext)
+
+      // Assert
+      expect(mockNext).toHaveBeenCalledWith()
+    })
+
+    it("blocks improper site names", async () => {
+      // Arrange
+      const mockInvalidReq = ({
+        params: { siteName: TEST_INVALID_SITE_NAME },
+      } as unknown) as Request<
+        never,
+        unknown,
+        unknown,
+        never,
+        { userWithSiteSessionData: UserWithSiteSessionData }
+      >
+
+      // Act
+      await routeCheckerMiddleware.verifySiteName(
+        mockInvalidReq,
+        mockRes,
+        mockNext
+      )
+
+      // Assert
+      expect(mockNext).toHaveBeenCalledWith(
+        new BadRequestError("Invalid site name")
+      )
+    })
+  })
+})

src/middleware/routeChecker.ts

@@ -0,0 +1,17 @@
+import { NextFunction, Request, Response } from "express"
+
+import { BadRequestError } from "@root/errors/BadRequestError"
+
+const SITE_NAME_REGEX = /^[a-zA-Z0-9-]+$/
+
+// eslint-disable-next-line import/prefer-default-export
+export class RouteCheckerMiddleware {
+  async verifySiteName(req: Request, _res: Response, next: NextFunction) {
+    const { params } = req
+    const { siteName } = params
+    if (siteName && !SITE_NAME_REGEX.test(siteName)) {
+      return next(new BadRequestError("Invalid site name"))
+    }
+    return next()
+  }
+}

src/routes/v2/authenticated/collaborators.ts

@@ -10,6 +10,7 @@ import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"
 import { BaseIsomerError } from "@root/errors/BaseError"
 import logger from "@root/logger/logger"
 import { attachSiteHandler } from "@root/middleware"
+import { RouteCheckerMiddleware } from "@root/middleware/routeChecker"
 import { RequestHandler } from "@root/types"
 import { UserDto } from "@root/types/dto/review"
 import CollaboratorsService from "@services/identity/CollaboratorsService"

src/routes/v2/authenticated/index.js

@@ -1,4 +1,5 @@
 import { attachSiteHandler } from "@root/middleware"
+import { RouteCheckerMiddleware } from "@root/middleware/routeChecker"
 
 import { MetricsRouter } from "./metrics"
 import { NotificationsRouter } from "./notifications"
@@ -45,6 +46,7 @@ const getAuthenticatedSubrouter = ({
     notificationsService,
   })
   const metricsRouter = new MetricsRouter({ authorizationMiddleware })
+  const routeCheckerMiddleware = new RouteCheckerMiddleware()
 
   const authenticatedSubrouter = express.Router({ mergeParams: true })
 
@@ -55,17 +57,20 @@ const getAuthenticatedSubrouter = ({
   authenticatedSubrouter.use("/metrics", metricsRouter.getRouter())
   authenticatedSubrouter.use(
     "/sites/:siteName/collaborators",
+    routeCheckerMiddleware.verifySiteName,
     collaboratorsRouter.getRouter()
   )
   const baseSitesV2Router = sitesV2Router.getRouter()
   const sitesRouterWithReviewRequest = baseSitesV2Router.use(
     "/:siteName/review",
+    routeCheckerMiddleware.verifySiteName,
     attachSiteHandler,
     reviewRouter.getRouter()
   )
   authenticatedSubrouter.use("/sites", sitesRouterWithReviewRequest)
   authenticatedSubrouter.use(
     "/sites/:siteName/notifications",
+    routeCheckerMiddleware.verifySiteName,
     notificationsRouter.getRouter()
   )
   authenticatedSubrouter.use("/user", usersRouter.getRouter())

src/routes/v2/authenticated/sites.ts

@@ -9,6 +9,7 @@ import UserWithSiteSessionData from "@classes/UserWithSiteSessionData"
 
 import type UserSessionData from "@root/classes/UserSessionData"
 import { attachSiteHandler } from "@root/middleware"
+import { RouteCheckerMiddleware } from "@root/middleware/routeChecker"
 import { StatsMiddleware } from "@root/middleware/stats"
 import InfraService from "@root/services/infra/InfraService"
 import type { RequestHandler } from "@root/types"
@@ -189,6 +190,7 @@ export class SitesRouter {
 
   getRouter() {
     const router = express.Router({ mergeParams: true })
+    const routeCheckerMiddleware = new RouteCheckerMiddleware()
 
     router.get(
       "/",
@@ -197,39 +199,46 @@ export class SitesRouter {
     )
     router.get(
       "/:siteName/lastUpdated",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       attachReadRouteHandlerWrapper(this.getLastUpdated)
     )
     router.get(
       "/:siteName/stagingUrl",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       attachReadRouteHandlerWrapper(this.getStagingUrl)
     )
     router.get(
       "/:siteName/siteUrl",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       attachReadRouteHandlerWrapper(this.getSiteUrl)
     )
     router.get(
       "/:siteName/info",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       this.authorizationMiddleware.verifySiteMember,
       attachReadRouteHandlerWrapper(this.getSiteInfo)
     )
     router.get(
       "/:siteName/launchInfo",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       this.authorizationMiddleware.verifySiteMember,
       attachReadRouteHandlerWrapper(this.getSiteLaunchInfo)
     )
     router.post(
       "/:siteName/launchSite",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       this.authorizationMiddleware.verifySiteAdmin,
       attachReadRouteHandlerWrapper(this.launchSite)
     )
     router.get(
       "/:siteName/getStagingBuildStatus",
+      routeCheckerMiddleware.verifySiteName,
       attachSiteHandler,
       this.authorizationMiddleware.verifySiteMember,
       attachReadRouteHandlerWrapper(this.getUserStagingSiteBuildStatus)

src/routes/v2/authenticatedSites/index.js

@@ -1,4 +1,5 @@
 import { attachSiteHandler } from "@root/middleware"
+import { RouteCheckerMiddleware } from "@root/middleware/routeChecker"
 
 const express = require("express")
 
@@ -199,9 +200,11 @@ const getAuthenticatedSitesSubrouter = ({
     repoManagementService,
     authorizationMiddleware,
   })
+  const routeCheckerMiddleware = new RouteCheckerMiddleware()
 
   const authenticatedSitesSubrouter = express.Router({ mergeParams: true })
 
+  authenticatedSitesSubrouter.use(routeCheckerMiddleware.verifySiteName)
   authenticatedSitesSubrouter.use(authenticationMiddleware.verifyAccess)
   authenticatedSitesSubrouter.use(attachSiteHandler)
   // NOTE: apiLogger needs to be after `verifyJwt` as it logs the github username