fix: reject prototype paths, fail closed on deny, and run unit tests in CI

.github/workflows/test.yml

@@ -0,0 +1,31 @@
+name: Test
+
+on:
+  pull_request:
+    types: [opened, synchronize]
+    branches:
+      - main
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v7
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v7
+        with:
+          node-version: 24
+
+      - name: Setup pnpm
+        uses: pnpm/action-setup@v6
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      - name: Test
+        run: pnpm test

permix/src/core/check.ts

@@ -42,6 +42,14 @@ export function callRuleWithoutData(rule: () => unknown): boolean {
   }
 }
 
+// Own-property lookup only, so paths like `post.constructor` or `toString`
+// never resolve through the prototype chain.
+function ownChild(parent: object, key: string): Rule | undefined {
+  return Object.hasOwn(parent, key)
+    ? (parent as Record<string, Rule>)[key]
+    : undefined
+}
+
 function walk(rules: Rules<any>, inputArgs: unknown[]): boolean {
   let args = inputArgs
   const first = args[0]
@@ -51,11 +59,12 @@ function walk(rules: Rules<any>, inputArgs: unknown[]): boolean {
     const last = parts.at(-1)
 
     if (isSpecialSymbol(last)) {
-      let subtree: Rule = rules
+      let subtree: Rule | undefined = rules
       for (let i = 0; i < parts.length - 1; i++) {
-        if (subtree && typeof subtree === 'object') {
-          subtree = (subtree as Record<string, Rule>)[parts[i]]
-        }
+        subtree =
+          subtree && typeof subtree === 'object'
+            ? ownChild(subtree, parts[i])
+            : undefined
       }
 
       if (subtree === undefined) {
@@ -71,11 +80,15 @@ function walk(rules: Rules<any>, inputArgs: unknown[]): boolean {
         if (typeof rule === 'function') {
           return void out.push(callRuleWithoutData(rule))
         }
-        for (const key in rule) {
+        for (const key of Object.keys(rule)) {
           visit(rule[key])
         }
       }
       visit(subtree)
+      // An empty subtree grants nothing, even for `~all`.
+      if (out.length === 0) {
+        return false
+      }
       return last === '~all' ? out.every(Boolean) : out.some(Boolean)
     }
 
@@ -84,10 +97,10 @@ function walk(rules: Rules<any>, inputArgs: unknown[]): boolean {
     }
   }
 
-  let rule: Rule = rules
+  let rule: Rule | undefined = rules
   let i = 0
   for (; i < args.length && typeof rule === 'object'; i++) {
-    rule = rule[String(args[i])]
+    rule = ownChild(rule, String(args[i]))
   }
 
   if (typeof rule === 'boolean') {

permix/src/core/permix.test.ts

@@ -757,3 +757,42 @@ describe('deep rules', () => {
     })
   })
 })
+
+describe('prototype safety', () => {
+  const permix = createPermix<{
+    post: ['create']
+  }>()
+
+  permix.setup({ post: { create: true } })
+
+  it('should not resolve inherited names as rules', () => {
+    // @ts-expect-error not a defined path
+    expect(() => permix.check('toString')).toThrow(PermixRuleNotDefinedError)
+    // @ts-expect-error not a defined path
+    expect(() => permix.check('post.constructor')).toThrow(
+      PermixRuleNotDefinedError
+    )
+    // @ts-expect-error not a defined path
+    expect(() => permix.check('constructor.~any')).toThrow(
+      PermixRuleNotDefinedError
+    )
+  })
+
+  it('should ignore __proto__ keys when hydrating', () => {
+    const state = JSON.parse(
+      '{"__proto__":{"admin":true},"post":{"create":false}}'
+    )
+    permix.hydrate(state)
+
+    expect(permix.check('post.create')).toBe(false)
+    expect(Object.getPrototypeOf(permix.getRules())).toBe(Object.prototype)
+    expect((permix.getRules() as any).admin).toBeUndefined()
+  })
+
+  it('should deny ~all on an empty subtree', () => {
+    permix.hydrate({ post: {} } as any)
+
+    expect(permix.check('post.~all')).toBe(false)
+    expect(permix.check('post.~any')).toBe(false)
+  })
+})

permix/src/core/rules.ts

@@ -60,7 +60,11 @@ export function hydrateRules<D extends Definition>(
   state: DehydratedState<D>
 ): Rules<D> {
   const result: Record<string, unknown> = {}
-  for (const key in state as Record<string, unknown>) {
+  for (const key of Object.keys(state)) {
+    // Untrusted JSON: assigning `__proto__` would swap the prototype.
+    if (key === '__proto__') {
+      continue
+    }
     const value = (state as Record<string, unknown>)[key]
     result[key] =
       typeof value === 'boolean'

permix/src/elysia/permix.test.ts

@@ -334,3 +334,28 @@ describe('key exposure', () => {
     expect(permix.key).toBeTypeOf('symbol')
   })
 })
+
+describe('fail closed', () => {
+  it('should respond 403 when a custom onForbidden returns nothing', async () => {
+    const permix = createPermix<PermissionsDefinition>({
+      onForbidden: () => {},
+    })
+
+    const app = new Elysia()
+      .onBeforeHandle(
+        permix.setupMiddleware({
+          post: { create: false, read: false, update: false },
+          user: { delete: false },
+        })
+      )
+      .post('/posts', () => ({ success: true }), {
+        beforeHandle: permix.checkMiddleware('post.create'),
+      })
+
+    const res = await app.handle(
+      new Request('http://localhost/posts', { method: 'POST' })
+    )
+    expect(res.status).toBe(403)
+    await expect(res.json()).resolves.toStrictEqual({ error: 'Forbidden' })
+  })
+})

permix/src/elysia/permix.ts

@@ -82,7 +82,16 @@ function buildPermix<D extends Definition>(
       const allowed = permix.check(...args)
 
       if (!allowed) {
-        return await onForbidden({ context, ...createCheckContext(...args) })
+        const result = await onForbidden({
+          context,
+          ...createCheckContext(...args),
+        })
+        // Fail closed if a custom handler returned nothing.
+        if (result === undefined) {
+          context.set.status = 'Forbidden'
+          return { error: 'Forbidden' }
+        }
+        return result
       }
     }
 

permix/src/express/permix.test.ts

@@ -497,3 +497,28 @@ describe('key exposure', () => {
     expect(permix.key).toBeTypeOf('symbol')
   })
 })
+
+describe('async errors', () => {
+  it('should forward a rejected setup callback to next(err)', async () => {
+    const permix = createPermix<PermissionsDefinition>()
+    const app = express()
+
+    app.use(
+      permix.setupMiddleware(async () => {
+        throw new Error('session lookup failed')
+      })
+    )
+    app.get('/', (_req, res) => {
+      res.json({ ok: true })
+    })
+
+    const errorHandler: ErrorRequestHandler = (err, _req, res, _next) => {
+      res.status(500).json({ error: err.message })
+    }
+    app.use(errorHandler)
+
+    const response = await request(app).get('/')
+    expect(response.status).toBe(500)
+    expect(response.body).toStrictEqual({ error: 'session lookup failed' })
+  })
+})

permix/src/express/permix.ts

@@ -60,15 +60,21 @@ function buildPermix<D extends Definition>(
       | Rules<D>
   ): Handler {
     return async (req, res, next) => {
-      const rules =
-        typeof callbackOrRules === 'function'
-          ? await callbackOrRules({ req, res, next })
-          : callbackOrRules
-      const instance = createPermixCore<D>(rules)
-      instance.hook('check', (context) => {
-        hooks.callHook('check', context)
-      })
-      ;(req as any)[resolveKey()] = instance
+      try {
+        const rules =
+          typeof callbackOrRules === 'function'
+            ? await callbackOrRules({ req, res, next })
+            : callbackOrRules
+        const instance = createPermixCore<D>(rules)
+        instance.hook('check', (context) => {
+          hooks.callHook('check', context)
+        })
+        ;(req as any)[resolveKey()] = instance
+      } catch (error) {
+        // Express 4 does not catch async rejections; forward them.
+        next(error)
+        return
+      }
       next()
     }
   }
@@ -83,15 +89,18 @@ function buildPermix<D extends Definition>(
         return
       }
 
-      const allowed = permix.check(...args)
-
-      if (!allowed) {
-        await onForbidden({
-          req,
-          res,
-          next,
-          ...createCheckContext(...args),
-        })
+      try {
+        if (!permix.check(...args)) {
+          await onForbidden({
+            req,
+            res,
+            next,
+            ...createCheckContext(...args),
+          })
+          return
+        }
+      } catch (error) {
+        next(error)
         return
       }
 

permix/src/fastify/permix.test.ts

@@ -385,3 +385,32 @@ describe('key exposure', () => {
     expect(permix.key).toBeTypeOf('symbol')
   })
 })
+
+describe('fail closed', () => {
+  it('should send 403 when a custom onForbidden does not reply', async () => {
+    const permix = createPermix<PermissionsDefinition>({
+      onForbidden: () => {},
+    })
+
+    const app = Fastify()
+
+    await app.register(
+      permix.setupMiddleware({
+        post: { create: false, read: false, update: false },
+        user: { delete: false },
+      })
+    )
+
+    app.post(
+      '/posts',
+      { preHandler: permix.checkMiddleware('post.create') },
+      (_req, reply) => {
+        reply.send({ success: true })
+      }
+    )
+
+    const response = await app.inject({ method: 'POST', url: '/posts' })
+    expect(response.statusCode).toBe(403)
+    expect(response.json()).toStrictEqual({ error: 'Forbidden' })
+  })
+})

permix/src/fastify/permix.ts

@@ -110,6 +110,10 @@ function buildPermix<D extends Definition>(
 
       if (!allowed) {
         await onForbidden({ request, reply, ...createCheckContext(...args) })
+        // Fail closed if a custom handler forgot to reply.
+        if (!reply.sent) {
+          reply.status(403).send({ error: 'Forbidden' })
+        }
       }
     }
 

permix/src/node/permix.test.ts

@@ -366,3 +366,20 @@ describe('key exposure', () => {
     expect(permix.key).toBeTypeOf('symbol')
   })
 })
+
+describe('async errors', () => {
+  it('should forward a rejected setup callback to next(err)', async () => {
+    const permix = createPermix<PermissionsDefinition>()
+    const req = createMockRequest()
+    const res = createMockResponse()
+    const next = createMockNext()
+    const error = new Error('session lookup failed')
+
+    await permix.setupMiddleware(async () => {
+      throw error
+    })(req, res, next)
+
+    expect(next).toHaveBeenCalledWith(error)
+    expect(permix.get(req)).toBeNull()
+  })
+})