Skip to content

fix(bearer-auth): escape regex metacharacters in bearer auth prefix option#4750

Merged
yusukebe merged 6 commits intohonojs:mainfrom
otoneko1102:fix/bearer-auth-prefix-regex-escape
Mar 13, 2026
Merged

fix(bearer-auth): escape regex metacharacters in bearer auth prefix option#4750
yusukebe merged 6 commits intohonojs:mainfrom
otoneko1102:fix/bearer-auth-prefix-regex-escape

Conversation

@otoneko1102
Copy link
Copy Markdown
Contributor

@otoneko1102 otoneko1102 commented Feb 22, 2026

The author should do the following, if applicable

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix to format the code
  • Add TSDoc/JSDoc to document the code

"prefix" option in Bearer Auth middleware is interpolated directly into new RegExp() without escaping. This may cause SyntaxError crashes or unintended pattern matching when "prefix" contains regex metacharacters.

e.g., (, ), ., +, ...

  • prefix: 'Bearer|Token' ...

Copy link
Copy Markdown
Contributor

@EdamAme-x EdamAme-x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@otoneko1102 Can you check this?

Comment thread src/middleware/bearer-auth/index.ts Outdated
Comment thread src/middleware/bearer-auth/index.ts Outdated
@otoneko1102 otoneko1102 changed the title fix: escape regex metacharacters in bearer auth prefix option fix(bearer-auth): escape regex metacharacters in bearer auth prefix option Feb 23, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.83%. Comparing base (2de30d7) to head (8ecfed6).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4750      +/-   ##
==========================================
+ Coverage   91.48%   92.83%   +1.35%     
==========================================
  Files         177      177              
  Lines       11556    11642      +86     
  Branches     3357     3466     +109     
==========================================
+ Hits        10572    10808     +236     
+ Misses        983      833     -150     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yusukebe
Copy link
Copy Markdown
Member

yusukebe commented Mar 9, 2026

Hey @otoneko1102

Thank you for the PR. I realized you don't need to use a regexp match for the prefix value. If so, the escapeRegExp in this PR is unnecessary, as shown by the following patch. The code is increased, but I think it is simple.

diff --git a/src/middleware/bearer-auth/index.ts b/src/middleware/bearer-auth/index.ts
index 3b9d225f..1a7e7854 100644
--- a/src/middleware/bearer-auth/index.ts
+++ b/src/middleware/bearer-auth/index.ts
@@ -13,7 +13,6 @@ const TOKEN_STRINGS = '[A-Za-z0-9._~+/-]+=*'
 const PREFIX = 'Bearer'
 const HEADER = 'Authorization'

-const escapeRegExp = (str: string): string => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
 type MessageFunction = (c: Context) => string | object | Promise<string | object>
 type CustomizedErrorResponseOptions = {
   wwwAuthenticateHeader?: string | object | MessageFunction
@@ -113,9 +112,9 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
   }

   const realm = options.realm?.replace(/"/g, '\\"')
-  const prefixRegexStr = options.prefix === '' ? '' : `${escapeRegExp(options.prefix)} +`
-  const regexp = new RegExp(`^${prefixRegexStr}(${TOKEN_STRINGS}) *$`, 'i')
-  const wwwAuthenticatePrefix = options.prefix === '' ? '' : `${options.prefix} `
+  const prefix = options.prefix
+  const tokenRegexp = new RegExp(`^${TOKEN_STRINGS}$`)
+  const wwwAuthenticatePrefix = prefix === '' ? '' : `${prefix} `

   const throwHTTPException = async (
     c: Context,
@@ -165,8 +164,19 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
           'Unauthorized'
       )
     } else {
-      const match = regexp.exec(headerToken)
-      if (!match) {
+      let tokenValue: string | undefined
+
+      if (prefix === '') {
+        tokenValue = headerToken
+      } else {
+        const headerLower = headerToken.toLowerCase()
+        const prefixLower = prefix.toLowerCase()
+        if (headerLower.startsWith(prefixLower) && headerToken[prefix.length] === ' ') {
+          tokenValue = headerToken.slice(prefix.length).trimStart()
+        }
+      }
+
+      if (!tokenValue || !tokenRegexp.test(tokenValue)) {
         // Invalid Request
         await throwHTTPException(
           c,
@@ -180,12 +190,12 @@ export const bearerAuth = (options: BearerAuthOptions): MiddlewareHandler => {
       } else {
         let equal = false
         if ('verifyToken' in options) {
-          equal = await options.verifyToken(match[1], c)
+          equal = await options.verifyToken(tokenValue, c)
         } else if (typeof options.token === 'string') {
-          equal = await timingSafeEqual(options.token, match[1], options.hashFunction)
+          equal = await timingSafeEqual(options.token, tokenValue, options.hashFunction)
         } else if (Array.isArray(options.token) && options.token.length > 0) {
           for (const token of options.token) {
-            if (await timingSafeEqual(token, match[1], options.hashFunction)) {
+            if (await timingSafeEqual(token, tokenValue, options.hashFunction)) {
               equal = true
               break
             }

@otoneko1102 @EdamAme-x @usualoma What do you think of this?

@otoneko1102
Copy link
Copy Markdown
Contributor Author

I agree with you

@yusukebe
Copy link
Copy Markdown
Member

@otoneko1102 Thanks! Can you change the code?

@otoneko1102
Copy link
Copy Markdown
Contributor Author

@yusukebe OK! I'll do it later.
Thanks for the review and advice!

@yusukebe
Copy link
Copy Markdown
Member

@otoneko1102 Thanks. Please ping me if it's done!

@otoneko1102
Copy link
Copy Markdown
Contributor Author

otoneko1102 commented Mar 12, 2026

@yusukebe Sorry for the delay! Fixed it!

An error has occurred for Node.js v20 test.
It is not a bug in the code, but the GitHub Actions call is failing.

@EdamAme-x
Copy link
Copy Markdown
Contributor

Looks good!

Copy link
Copy Markdown
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe
Copy link
Copy Markdown
Member

@otoneko1102 @EdamAme-x

The CI works well now. Looks good! Thanks!

@yusukebe yusukebe merged commit 0c0bf8d into honojs:main Mar 13, 2026
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants