fix(middleware/jwt): fix incorrect assumption in jwt impl

[boehs]

The author should do the following, if applicable

Please see https://discord.com/channels/595317990191398933/1243024990652207115/1243024990652207115 (in the cloudflare discord)

THIS WILL BREAK PRODUCTION

In the docs it says

app.use(
  '/auth/*',
  jwt({
    secret: 'it-is-very-secret',
  })
)
app.get('/auth/page', (c) => {
  return c.text('You are authorized')
})

realalitically code in production will look more like

app.use(
  '/auth/*',
  jwt({
    secret: process.env.SECRET,
  })
)
app.get('/auth/page', (c) => {
  return c.text('You are authorized')
})

however, if this app.use is at the top level, in some instances in cloudflare process.env can be undefined. Nobody noticed this, because the code failed to check if it was actually defined.

edit: another solution would be to move this check inside the return callback, but this means that every API call it would run the getter on process.env which would make it susceptible to runtime manipulation.

Though actually if my theory that this is an issue is correct, in its current impl where the check is running before the callback is constructed, it is still vulnerable to runtime manipulation without reruning the check because it never does let secret = options.secret so if you could modify the value of secret (eg if it is a reference to process.env.secret and you modify it elsewhere) but actually, I think I'm wrong because when you construct an object like let obj = { secret: process.env.SECRET }, changing process.env wouldn't have an impact because when SECRET is accessed thats no longer a reference type, thats a copy.

HOWEVER if you access process.env.SECRET every single request, like when I proposed

edit: another solution would be to move this

it would hit the ref type every single request, so that would make it a vuln

• Add tests
• Run tests
• bun denoify to generate files for Deno
• bun run format:fix && bun run lint:fix to format the code

[yusukebe]

Hi @boehs

Thanks for the PR. You are right. options.secret should be checked first.

I think this will not be a "breaking change." It just changes the internal behavior in the JWT middleware, so we can ship it as a patch or minor release.

One thing. Could you run the bun run denoify to generate files for Deno and push it?

[boehs]

Hi @boehs

Thanks for the PR. You are right. options.secret should be checked first.

I think this will not be a "breaking change." It just changes the internal behavior in the JWT middleware, so we can ship it as a patch or minor release.

yeah the breaking assumption was before I tried to pentest it and found it wasn't working with undefined and discovered that by luck there was an exception. I'll denoify it now

[boehs]

done

[yusukebe]

@boehs

Thank you! I'll merge this now and will ship it.