fix: replace koa-helmet (#379)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Chores**
  * Updated security middleware dependency to version 8.1.0.
  * Removed legacy dependency.

* **Refactor**
* Restructured middleware configuration for improved framework
compatibility.

* **Tests**
  * Updated test suite to reflect middleware changes.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: csikb

package.json

@@ -20,12 +20,12 @@
   "//": "Dependencies required at runtime",
   "dependencies": {
     "@graphile-contrib/pg-simplify-inflector": "6.1.0",
-    "@koa/router": "15.0.0",
     "@koa/bodyparser": "6.0.0",
+    "@koa/router": "15.0.0",
     "dotenv": "17.2.3",
+    "helmet": "8.1.0",
     "koa": "3.1.1",
     "koa-compress": "5.1.1",
-    "koa-helmet": "8.0.1",
     "pg": "8.16.3",
     "postgraphile": "4.14.1"
   },

pnpm-lock.yaml

@@ -4,7 +4,7 @@ import config from './config.js'
 import {
   bodyParser,
   compress,
-  helmet,
+  koaHelmet,
   postGraphile,
 } from './middleware/index.js'
 import { healthRouter } from './router/index.js'
@@ -27,7 +27,10 @@ vi.mock('./config.js', () => ({
 vi.mock('./middleware/index.js', () => ({
   bodyParser: vi.fn().mockName('bodyParser'),
   compress: vi.fn().mockName('compress'),
-  helmet: vi.fn().mockName('helmet'),
+  koaHelmet: vi
+    .fn()
+    .mockName('koaHelmet')
+    .mockReturnValue(vi.fn().mockName('helmet-middleware')),
   postGraphile: vi.fn().mockName('postGraphile'),
 }))
 vi.mock('./router/index.js', () => ({
@@ -43,7 +46,7 @@ describe('index', () => {
   })
 
   it('should be tested', async () => {
-    expect.assertions(12)
+    expect.assertions(13)
 
     await import('./index.js')
 
@@ -53,7 +56,8 @@ describe('index', () => {
     expect.soft(mockKoaInstance.use).toHaveBeenCalledTimes(6)
     expect.soft(mockKoaInstance.use).toHaveBeenCalledWith(bodyParser)
     expect.soft(mockKoaInstance.use).toHaveBeenCalledWith(compress)
-    expect.soft(mockKoaInstance.use).toHaveBeenCalledWith(helmet)
+    expect.soft(koaHelmet).toHaveBeenCalledWith()
+    expect.soft(mockKoaInstance.use).toHaveBeenCalledWith(koaHelmet())
     expect.soft(healthRouter.routes).toHaveBeenCalledOnce()
     expect.soft(mockKoaInstance.use).toHaveBeenCalledWith(healthRouter.routes())
     expect.soft(healthRouter.allowedMethods).toHaveBeenCalledOnce()

src/index.test.ts

@@ -3,7 +3,7 @@ import config from './config.js'
 import {
   bodyParser,
   compress,
-  helmet,
+  koaHelmet,
   postGraphile,
 } from './middleware/index.js'
 import { healthRouter } from './router/index.js'
@@ -14,7 +14,7 @@ app
   // register common middleware
   .use(bodyParser)
   .use(compress)
-  .use(helmet)
+  .use(koaHelmet())
   // register health router
   .use(healthRouter.routes())
   .use(healthRouter.allowedMethods())

src/index.ts

@@ -1,15 +1,97 @@
-import helmet from 'koa-helmet'
-import { describe, expect, it, vi } from 'vitest'
+import helmet from 'helmet'
+import type { Context } from 'koa'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-vi.mock('koa-helmet')
+vi.mock('helmet')
 
 describe('helmet', () => {
-  it('should export helmet', async () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+  })
+
+  it('should export a factory that calls helmet and returns Koa middleware', async () => {
+    expect.assertions(3)
+
+    const mockExpressHelmet = vi.fn()
+    const mockHelmet = vi.mocked(helmet)
+    mockHelmet.mockReturnValue(mockExpressHelmet)
+
+    const { koaHelmet } = await import('./helmet.js')
+
+    const middleware = koaHelmet()
+
+    expect(mockHelmet).toHaveBeenCalledOnce()
+    expect(mockHelmet).toHaveBeenCalledWith(undefined)
+    expect(middleware).toBeTypeOf('function')
+  })
+
+  it('should call helmet with options when provided', async () => {
+    expect.assertions(3)
+
+    const mockExpressHelmet = vi.fn()
+    const mockHelmet = vi.mocked(helmet)
+    mockHelmet.mockReturnValue(mockExpressHelmet)
+
+    const { koaHelmet } = await import('./helmet.js')
+
+    const options = { contentSecurityPolicy: false }
+    const middleware = koaHelmet(options)
+
+    expect(mockHelmet).toHaveBeenCalledOnce()
+    expect(mockHelmet).toHaveBeenCalledWith(options)
+    expect(middleware).toBeTypeOf('function')
+  })
+
+  it('should call the express helmet middleware and continue to next', async () => {
     expect.assertions(2)
 
-    const { default: actual } = await import('./helmet.js')
+    const mockExpressHelmet = vi.fn((_req, _res, next) => {
+      next()
+    })
+    const mockHelmet = vi.mocked(helmet)
+    mockHelmet.mockReturnValue(mockExpressHelmet)
+
+    const { koaHelmet } = await import('./helmet.js')
+
+    const middleware = koaHelmet()
+
+    const ctx = {
+      req: {},
+      res: {},
+    } as Context
+    const next = vi.fn().mockResolvedValue(undefined)
+
+    await middleware(ctx, next)
+
+    expect(mockExpressHelmet).toHaveBeenCalledWith(
+      ctx.req,
+      ctx.res,
+      expect.any(Function),
+    )
+    expect(next).toHaveBeenCalledOnce()
+  })
+
+  it('should reject promise when express helmet middleware returns error', async () => {
+    expect.assertions(2)
+
+    const error = new Error('Helmet error')
+    const mockExpressHelmet = vi.fn((_req, _res, callback) => {
+      callback(error)
+    })
+    const mockHelmet = vi.mocked(helmet)
+    mockHelmet.mockReturnValue(mockExpressHelmet)
+
+    const { koaHelmet } = await import('./helmet.js')
+
+    const middleware = koaHelmet()
+
+    const ctx = {
+      req: {},
+      res: {},
+    } as Context
+    const next = vi.fn()
 
-    expect(helmet.default).toHaveBeenCalledOnce()
-    expect(actual).toStrictEqual(helmet.default())
+    await expect(middleware(ctx, next)).rejects.toThrow('Helmet error')
+    expect(next).not.toHaveBeenCalled()
   })
 })

src/middleware/helmet.test.ts

@@ -1,3 +1,20 @@
-import helmet from 'koa-helmet'
+import helmet, { type HelmetOptions } from 'helmet'
+import type { Middleware } from 'koa'
 
-export default helmet.default()
+/**
+ * Koa-compatible Helmet middleware.
+ *
+ * Accepts the same options helmet() does.
+ */
+export function koaHelmet(options?: HelmetOptions): Middleware {
+  const expressHelmet = helmet(options)
+  return async (ctx, next) => {
+    await new Promise<void>((resolve, reject) => {
+      expressHelmet(ctx.req, ctx.res, (err) => {
+        if (err) return reject(err)
+        resolve()
+      })
+    })
+    return next()
+  }
+}

src/middleware/helmet.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it, vi } from 'vitest'
 import bodyParser from './bodyparser.js'
 import compress from './compress.js'
-import helmet from './helmet.js'
+import { koaHelmet } from './helmet.js'
 import postGraphile from './postgraphile.js'
 
 vi.mock('./bodyparser', () => ({
@@ -11,7 +11,7 @@ vi.mock('./compress', () => ({
   default: vi.fn().mockName('compress'),
 }))
 vi.mock('./helmet', () => ({
-  default: vi.fn().mockName('helmet'),
+  koaHelmet: vi.fn().mockName('helmet'),
 }))
 vi.mock('./postgraphile', () => ({
   default: vi.fn().mockName('postgraphile'),
@@ -26,10 +26,12 @@ describe('index', () => {
     const expected = {
       bodyParser,
       compress,
-      helmet,
+      koaHelmet,
       postGraphile,
     }
     expect.soft(actual).toMatchObject(expected)
-    expect.soft(Object.keys(actual)).toEqual(Object.keys(expected))
+    expect
+      .soft(Object.keys(actual).sort())
+      .toEqual(Object.keys(expected).sort())
   })
 })

src/middleware/index.test.ts

@@ -1,4 +1,4 @@
 export { default as bodyParser } from './bodyparser.js'
 export { default as compress } from './compress.js'
-export { default as helmet } from './helmet.js'
+export * from './helmet.js'
 export { default as postGraphile } from './postgraphile.js'

src/middleware/index.ts

@@ -17,7 +17,6 @@ router.get('/health', async (ctx) => {
       }
       ctx.body = 'ERROR: no views found in database'
       ctx.status = 500
-      return
     })
     .catch(() => {
       ctx.body = 'ERROR: cannot connect to database'