From c9d5bbb1c03beabdc1dddcd7a273393aac55507e Mon Sep 17 00:00:00 2001 From: James Date: Fri, 10 Jul 2026 12:21:22 +0100 Subject: [PATCH 1/9] fix(middleware): support mixed and repeating matchers --- packages/vinext/src/index.ts | 7 + .../src/server/middleware-matcher-pattern.ts | 109 +++++++ .../vinext/src/server/middleware-matcher.ts | 81 +---- .../src/server/middleware-path-to-regexp.ts | 281 +++++++++++++++++ .../app/admin/secrets/page.tsx | 3 + .../app/bar/secret/page.tsx | 3 + .../app/dashboard/users/page.tsx | 3 + .../middleware-matcher-auth/app/de/page.tsx | 3 + .../app/docs/[[...lang]]/page.tsx | 3 + .../app/en/profile/page.tsx | 3 + .../middleware-matcher-auth/app/layout.tsx | 7 + .../app/manual/[...lang]/page.tsx | 3 + .../app/public/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 19 ++ .../middleware-matcher-auth/package.json | 5 + .../middleware-matcher-auth/wrangler.jsonc | 10 + .../middleware-matcher-redos-child.ts | 10 + tests/middleware-matcher-auth.test.ts | 290 ++++++++++++++++++ tests/shims.test.ts | 132 +++++++- 19 files changed, 894 insertions(+), 81 deletions(-) create mode 100644 packages/vinext/src/server/middleware-matcher-pattern.ts create mode 100644 packages/vinext/src/server/middleware-path-to-regexp.ts create mode 100644 tests/fixtures/middleware-matcher-auth/app/admin/secrets/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/bar/secret/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/dashboard/users/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/de/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/docs/[[...lang]]/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/en/profile/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/layout.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/manual/[...lang]/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/public/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/middleware.ts create mode 100644 tests/fixtures/middleware-matcher-auth/package.json create mode 100644 tests/fixtures/middleware-matcher-auth/wrangler.jsonc create mode 100644 tests/fixtures/middleware-matcher-redos-child.ts create mode 100644 tests/middleware-matcher-auth.test.ts diff --git a/packages/vinext/src/index.ts b/packages/vinext/src/index.ts index 026947e709..443ac40aa3 100644 --- a/packages/vinext/src/index.ts +++ b/packages/vinext/src/index.ts @@ -85,6 +85,7 @@ import { import { mergeServerExternalPackages } from "./config/server-external-packages.js"; import { findMiddlewareFile, isProxyFile, runMiddleware } from "./server/middleware.js"; +import { validateMiddlewareMatcherPatterns } from "./server/middleware-matcher-pattern.js"; import { isNextDataPathname, normalizeNextDataPagePathname, @@ -2009,6 +2010,12 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { ? path.join(root, "src") : root; middlewarePath = findMiddlewareFile(root, fileMatcher, middlewareConventionDir); + if (middlewarePath) { + const staticMatcher = extractMiddlewareMatcherConfig(middlewarePath); + if (staticMatcher !== undefined) { + validateMiddlewareMatcherPatterns(staticMatcher); + } + } const instrumentationClientInjects = nextConfig.instrumentationClientInject.map((spec) => spec.startsWith("./") || spec.startsWith("../") ? path.resolve(root, spec) : spec, ); diff --git a/packages/vinext/src/server/middleware-matcher-pattern.ts b/packages/vinext/src/server/middleware-matcher-pattern.ts new file mode 100644 index 0000000000..130524f393 --- /dev/null +++ b/packages/vinext/src/server/middleware-matcher-pattern.ts @@ -0,0 +1,109 @@ +import { isSafeRegex } from "../config/config-matchers.js"; +import { + middlewarePathTokensToRegExp, + normalizeMiddlewarePathTokens, + parseMiddlewarePath, + type MiddlewarePathKey, + type MiddlewarePathToken, +} from "./middleware-path-to-regexp.js"; + +export type CompiledMiddlewareMatcherPattern = + | { regexp: RegExp; error?: never } + | { regexp?: never; error: string; kind: "invalid" | "unsafe" }; + +function patternMatches(pattern: string, value: string): boolean { + try { + return new RegExp(`^(?:${pattern})$`).test(value); + } catch { + return false; + } +} + +function unsafeTokenReason(token: MiddlewarePathKey): string | null { + if (!isSafeRegex(token.pattern)) { + return `parameter "${token.name}" contains nested repetition`; + } + + if (token.modifier !== "*" && token.modifier !== "+") return null; + + // Repeating parameters are joined by the token prefix/suffix. If their own + // constraint can also consume a slash (or the empty string), the same input + // has many equivalent partitions. Patterns such as `/:path(.*)*/end` then + // backtrack exponentially on a near miss. Ordinary repeats use a constraint + // that cannot cross the path delimiter, so each segment has one owner. + if (patternMatches(token.pattern, "") || patternMatches(token.pattern, "/")) { + return `repeated parameter "${token.name}" may match an empty value or path delimiter`; + } + + return null; +} + +function validateTokens(tokens: MiddlewarePathToken[]): string | null { + for (const token of tokens) { + if (typeof token === "string") continue; + const reason = unsafeTokenReason(token); + if (reason) return reason; + } + return null; +} + +export function compileMiddlewareMatcherPattern(source: string): CompiledMiddlewareMatcherPattern { + if (!source.startsWith("/")) { + return { kind: "invalid", error: "source must start with /" }; + } + if (source.length > 4096) { + return { kind: "invalid", error: "source exceeds max built length of 4096" }; + } + + let tokens: MiddlewarePathToken[]; + try { + tokens = parseMiddlewarePath(source); + } catch (error) { + return { + kind: "invalid", + error: error instanceof Error ? error.message : "matcher could not be parsed", + }; + } + + const unsafeReason = validateTokens(tokens); + if (unsafeReason) return { kind: "unsafe", error: unsafeReason }; + + try { + return { regexp: middlewarePathTokensToRegExp(tokens) }; + } catch { + // Match Next.js 16.2.7's path-to-regexp 6.3 normalization: repeating + // tokens without a prefix/suffix receive a slash prefix and are retried. + const normalizedTokens = normalizeMiddlewarePathTokens(tokens); + const normalizedUnsafeReason = validateTokens(normalizedTokens); + if (normalizedUnsafeReason) return { kind: "unsafe", error: normalizedUnsafeReason }; + try { + return { regexp: middlewarePathTokensToRegExp(normalizedTokens) }; + } catch (error) { + return { + kind: "invalid", + error: error instanceof Error ? error.message : "matcher could not be compiled", + }; + } + } +} + +export function validateMiddlewareMatcherPatterns(value: unknown): void { + const sources: string[] = []; + if (typeof value === "string") { + sources.push(value); + } else if (Array.isArray(value)) { + for (const matcher of value) { + if (typeof matcher === "string") sources.push(matcher); + else if (matcher && typeof matcher === "object" && "source" in matcher) { + const source = Reflect.get(matcher, "source"); + if (typeof source === "string") sources.push(source); + } + } + } + + for (const source of sources) { + const result = compileMiddlewareMatcherPattern(source); + if (result.regexp) continue; + throw new Error(`Invalid middleware matcher "${source}": ${result.error}.`); + } +} diff --git a/packages/vinext/src/server/middleware-matcher.ts b/packages/vinext/src/server/middleware-matcher.ts index 0699800576..0a1b05c21d 100644 --- a/packages/vinext/src/server/middleware-matcher.ts +++ b/packages/vinext/src/server/middleware-matcher.ts @@ -1,12 +1,11 @@ import { checkHasConditions, - isSafeRegex, requestContextFromRequest, - safeRegExp, type RequestContext, } from "../config/config-matchers.js"; import type { HasCondition, NextI18nConfig } from "../config/next-config.js"; import { removeTrailingSlash } from "../utils/base-path.js"; +import { compileMiddlewareMatcherPattern } from "./middleware-matcher-pattern.js"; type MiddlewareMatcherObject = { source: string; @@ -25,7 +24,7 @@ const EMPTY_MIDDLEWARE_REQUEST_CONTEXT: RequestContext = { }; const UNSAFE_MATCHER_PATTERN = Symbol("unsafe matcher pattern"); -type CompiledMatcherPattern = RegExp | null | typeof UNSAFE_MATCHER_PATTERN; +type CompiledMatcherPattern = RegExp | typeof UNSAFE_MATCHER_PATTERN; const _mwPatternCache = new Map(); @@ -136,75 +135,19 @@ export function matchPattern(pathname: string, pattern: string): boolean { _mwPatternCache.set(normalizedPattern, cached); } if (cached === UNSAFE_MATCHER_PATTERN) return true; - if (cached === null) return removeTrailingSlash(pathname) === normalizedPattern; if (cached.test(pathname)) return true; return pathname.endsWith("/") && cached.test(removeTrailingSlash(pathname)); } -function extractConstraint(str: string, re: RegExp): string | null { - if (str[re.lastIndex] !== "(") return null; - const start = re.lastIndex + 1; - let depth = 1; - let i = start; - while (i < str.length && depth > 0) { - if (str[i] === "(") depth++; - else if (str[i] === ")") depth--; - i++; - } - if (depth !== 0) return null; - re.lastIndex = i; - return str.slice(start, i - 1); -} - function compileMatcherPattern(pattern: string): CompiledMatcherPattern { - const hasConstraints = /:[\w-]+[*+]?\(/.test(pattern); - - if (!hasConstraints && (pattern.includes("(") || pattern.includes("\\"))) { - return compileMatcherRegExp("^" + pattern + "$", pattern); - } - - let regexStr = ""; - const tokenRe = /\/:([\w-]+)\*|\/:([\w-]+)\+|:([\w-]+)|[.]|[^/:.]+|./g; - let tok: RegExpExecArray | null; - while ((tok = tokenRe.exec(pattern)) !== null) { - if (tok[1] !== undefined) { - const constraint = hasConstraints ? extractConstraint(pattern, tokenRe) : null; - regexStr += constraint !== null ? `(?:/(${constraint}))?` : "(?:/.*)?"; - } else if (tok[2] !== undefined) { - const constraint = hasConstraints ? extractConstraint(pattern, tokenRe) : null; - regexStr += constraint !== null ? `(?:/(${constraint}))` : "(?:/.+)"; - } else if (tok[3] !== undefined) { - const constraint = hasConstraints ? extractConstraint(pattern, tokenRe) : null; - const isOptional = pattern[tokenRe.lastIndex] === "?"; - if (isOptional) tokenRe.lastIndex += 1; - - const group = constraint !== null ? `(${constraint})` : "([^/]+)"; - - if (isOptional && regexStr.endsWith("/")) { - regexStr = regexStr.slice(0, -1) + `(?:/${group})?`; - } else if (isOptional) { - regexStr += `${group}?`; - } else { - regexStr += group; - } - } else if (tok[0] === ".") { - regexStr += "\\."; - } else { - regexStr += tok[0]; - } - } - - return compileMatcherRegExp("^" + regexStr + "$", pattern); -} - -function compileMatcherRegExp(regexPattern: string, sourcePattern: string): CompiledMatcherPattern { - if (!isSafeRegex(regexPattern)) { - console.warn( - `[vinext] Rejecting potentially unsafe middleware matcher (ReDoS risk): ${sourcePattern}\n` + - ` Middleware will run for all paths to avoid bypassing request guards.\n` + - ` Simplify the matcher to avoid nested repetition.`, - ); - return UNSAFE_MATCHER_PATTERN; - } - return safeRegExp(regexPattern); + const result = compileMiddlewareMatcherPattern(pattern); + if (result.regexp) return result.regexp; + + const problem = result.kind === "unsafe" ? "potentially unsafe" : "invalid"; + console.warn( + `[vinext] Rejecting ${problem} middleware matcher: ${pattern}\n` + + ` ${result.error}.\n` + + ` Middleware will run for all paths to avoid bypassing request guards.`, + ); + return UNSAFE_MATCHER_PATTERN; } diff --git a/packages/vinext/src/server/middleware-path-to-regexp.ts b/packages/vinext/src/server/middleware-path-to-regexp.ts new file mode 100644 index 0000000000..2f5e0133df --- /dev/null +++ b/packages/vinext/src/server/middleware-path-to-regexp.ts @@ -0,0 +1,281 @@ +// Derived from path-to-regexp 6.3.0: +// https://github.com/pillarjs/path-to-regexp/tree/v6.3.0 +// +// The MIT License (MIT) +// Copyright (c) 2014 Blake Embrey (hello@blakeembrey.com) +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +export type MiddlewarePathToken = string | MiddlewarePathKey; + +export type MiddlewarePathKey = { + name: string | number; + prefix: string; + suffix: string; + pattern: string; + modifier: string; +}; + +type LexerToken = { + type: "MODIFIER" | "ESCAPED_CHAR" | "OPEN" | "CLOSE" | "NAME" | "PATTERN" | "CHAR" | "END"; + index: number; + value: string; +}; + +function lexer(value: string): LexerToken[] { + const tokens: LexerToken[] = []; + let index = 0; + + while (index < value.length) { + const character = value[index]; + if (character === "*" || character === "+" || character === "?") { + tokens.push({ type: "MODIFIER", index, value: value[index++] }); + continue; + } + if (character === "\\") { + tokens.push({ type: "ESCAPED_CHAR", index: index++, value: value[index++] }); + continue; + } + if (character === "{") { + tokens.push({ type: "OPEN", index, value: value[index++] }); + continue; + } + if (character === "}") { + tokens.push({ type: "CLOSE", index, value: value[index++] }); + continue; + } + if (character === ":") { + let name = ""; + let nameEnd = index + 1; + while (nameEnd < value.length) { + const code = value.charCodeAt(nameEnd); + if ( + (code >= 48 && code <= 57) || + (code >= 65 && code <= 90) || + (code >= 97 && code <= 122) || + code === 95 + ) { + name += value[nameEnd++]; + continue; + } + break; + } + if (!name) throw new TypeError(`Missing parameter name at ${index}`); + tokens.push({ type: "NAME", index, value: name }); + index = nameEnd; + continue; + } + if (character === "(") { + let depth = 1; + let pattern = ""; + let patternEnd = index + 1; + if (value[patternEnd] === "?") { + throw new TypeError(`Pattern cannot start with "?" at ${patternEnd}`); + } + while (patternEnd < value.length) { + if (value[patternEnd] === "\\") { + pattern += value[patternEnd++] + value[patternEnd++]; + continue; + } + if (value[patternEnd] === ")") { + depth--; + if (depth === 0) { + patternEnd++; + break; + } + } else if (value[patternEnd] === "(") { + depth++; + if (value[patternEnd + 1] !== "?") { + throw new TypeError(`Capturing groups are not allowed at ${patternEnd}`); + } + } + pattern += value[patternEnd++]; + } + if (depth) throw new TypeError(`Unbalanced pattern at ${index}`); + if (!pattern) throw new TypeError(`Missing pattern at ${index}`); + tokens.push({ type: "PATTERN", index, value: pattern }); + index = patternEnd; + continue; + } + tokens.push({ type: "CHAR", index, value: value[index++] }); + } + + tokens.push({ type: "END", index, value: "" }); + return tokens; +} + +export function parseMiddlewarePath(value: string): MiddlewarePathToken[] { + const tokens = lexer(value); + const result: MiddlewarePathToken[] = []; + const prefixes = "./"; + const delimiter = "/#?"; + let key = 0; + let index = 0; + let path = ""; + + const tryConsume = (type: LexerToken["type"]): string | undefined => { + if (index < tokens.length && tokens[index].type === type) { + return tokens[index++].value; + } + return undefined; + }; + + const mustConsume = (type: LexerToken["type"]): string => { + const consumed = tryConsume(type); + if (consumed !== undefined) return consumed; + const next = tokens[index]; + throw new TypeError(`Unexpected ${next.type} at ${next.index}, expected ${type}`); + }; + + const consumeText = (): string => { + let text = ""; + let consumed: string | undefined; + while ((consumed = tryConsume("CHAR") ?? tryConsume("ESCAPED_CHAR")) !== undefined) { + text += consumed; + } + return text; + }; + + const containsDelimiter = (text: string): boolean => { + for (const character of delimiter) { + if (text.includes(character)) return true; + } + return false; + }; + + const defaultPattern = (prefix: string): string => { + const previous = result[result.length - 1]; + const previousText = prefix || (typeof previous === "string" ? previous : ""); + if (previous && !previousText) { + const name = typeof previous === "string" ? previous : previous.name; + throw new TypeError(`Must have text between two parameters, missing text after "${name}"`); + } + if (!previousText || containsDelimiter(previousText)) return "[^\\/#\\?]+?"; + return `(?:(?!${escapeRegex(previousText)})[^\\/#\\?])+?`; + }; + + while (index < tokens.length) { + const character = tryConsume("CHAR"); + const name = tryConsume("NAME"); + const pattern = tryConsume("PATTERN"); + if (name !== undefined || pattern !== undefined) { + let prefix = character ?? ""; + if (!prefixes.includes(prefix)) { + path += prefix; + prefix = ""; + } + if (path) { + result.push(path); + path = ""; + } + result.push({ + name: name ?? key++, + prefix, + suffix: "", + pattern: pattern ?? defaultPattern(prefix), + modifier: tryConsume("MODIFIER") ?? "", + }); + continue; + } + + const text = character ?? tryConsume("ESCAPED_CHAR"); + if (text !== undefined) { + path += text; + continue; + } + if (path) { + result.push(path); + path = ""; + } + if (tryConsume("OPEN") !== undefined) { + const prefix = consumeText(); + const groupName = tryConsume("NAME") ?? ""; + const groupPattern = tryConsume("PATTERN") ?? ""; + const suffix = consumeText(); + mustConsume("CLOSE"); + result.push({ + name: groupName || (groupPattern ? key++ : ""), + pattern: groupName && !groupPattern ? defaultPattern(prefix) : groupPattern, + prefix, + suffix, + modifier: tryConsume("MODIFIER") ?? "", + }); + continue; + } + mustConsume("END"); + } + + return result; +} + +function escapeRegex(value: string): string { + return value.replace(/([.+*?=^!:${}()[\]|/\\])/g, "\\$1"); +} + +export function normalizeMiddlewarePathTokens( + tokens: MiddlewarePathToken[], +): MiddlewarePathToken[] { + return tokens.map((token) => { + if ( + typeof token === "object" && + (token.modifier === "*" || token.modifier === "+") && + token.prefix === "" && + token.suffix === "" + ) { + return { ...token, prefix: "/" }; + } + return token; + }); +} + +export function middlewarePathTokensToRegExp(tokens: MiddlewarePathToken[]): RegExp { + const delimiter = "/#?"; + const delimiterRegex = `[${escapeRegex(delimiter)}]`; + let route = "^"; + + for (const token of tokens) { + if (typeof token === "string") { + route += escapeRegex(token); + continue; + } + + const prefix = escapeRegex(token.prefix); + const suffix = escapeRegex(token.suffix); + if (token.pattern) { + if (prefix || suffix) { + if (token.modifier === "+" || token.modifier === "*") { + const optional = token.modifier === "*" ? "?" : ""; + route += `(?:${prefix}((?:${token.pattern})(?:${suffix}${prefix}(?:${token.pattern}))*)${suffix})${optional}`; + } else { + route += `(?:${prefix}(${token.pattern})${suffix})${token.modifier}`; + } + } else { + if (token.modifier === "+" || token.modifier === "*") { + throw new TypeError(`Can not repeat "${token.name}" without a prefix and suffix`); + } + route += `(${token.pattern})${token.modifier}`; + } + } else { + route += `(?:${prefix}${suffix})${token.modifier}`; + } + } + + route += `${delimiterRegex}?$`; + return new RegExp(route, "i"); +} diff --git a/tests/fixtures/middleware-matcher-auth/app/admin/secrets/page.tsx b/tests/fixtures/middleware-matcher-auth/app/admin/secrets/page.tsx new file mode 100644 index 0000000000..620b0e8c91 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/admin/secrets/page.tsx @@ -0,0 +1,3 @@ +export default function AdminSecretsPage() { + return
admin secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/bar/secret/page.tsx b/tests/fixtures/middleware-matcher-auth/app/bar/secret/page.tsx new file mode 100644 index 0000000000..d2c2d8d3d3 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/bar/secret/page.tsx @@ -0,0 +1,3 @@ +export default function MixedGroupPage() { + return
mixed group secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/dashboard/users/page.tsx b/tests/fixtures/middleware-matcher-auth/app/dashboard/users/page.tsx new file mode 100644 index 0000000000..c5afba7257 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/dashboard/users/page.tsx @@ -0,0 +1,3 @@ +export default function DashboardUsersPage() { + return
dashboard users secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/de/page.tsx b/tests/fixtures/middleware-matcher-auth/app/de/page.tsx new file mode 100644 index 0000000000..d3dd202fc4 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/de/page.tsx @@ -0,0 +1,3 @@ +export default function GermanPage() { + return
german locale secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/docs/[[...lang]]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/docs/[[...lang]]/page.tsx new file mode 100644 index 0000000000..d3ac57892c --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/docs/[[...lang]]/page.tsx @@ -0,0 +1,3 @@ +export default function DocsPage() { + return
docs secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/en/profile/page.tsx b/tests/fixtures/middleware-matcher-auth/app/en/profile/page.tsx new file mode 100644 index 0000000000..0a21ef38f4 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/en/profile/page.tsx @@ -0,0 +1,3 @@ +export default function EnglishProfilePage() { + return
english profile secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/layout.tsx b/tests/fixtures/middleware-matcher-auth/app/layout.tsx new file mode 100644 index 0000000000..f3ef34cd8b --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/layout.tsx @@ -0,0 +1,7 @@ +export default function RootLayout({ children }: { children: React.ReactNode }) { + return ( + + {children} + + ); +} diff --git a/tests/fixtures/middleware-matcher-auth/app/manual/[...lang]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/manual/[...lang]/page.tsx new file mode 100644 index 0000000000..c6e43d1473 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/manual/[...lang]/page.tsx @@ -0,0 +1,3 @@ +export default function ManualPage() { + return
manual secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/public/page.tsx b/tests/fixtures/middleware-matcher-auth/app/public/page.tsx new file mode 100644 index 0000000000..9ed6396f07 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/public/page.tsx @@ -0,0 +1,3 @@ +export default function PublicPage() { + return
public page
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts new file mode 100644 index 0000000000..a4f2356ad2 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -0,0 +1,19 @@ +import type { NextRequest } from "next/server"; + +export function middleware(_request: NextRequest) { + return new Response("blocked by middleware", { + status: 403, + headers: { "x-auth-guard": "blocked" }, + }); +} + +export const config = { + matcher: [ + "/(admin|dashboard)/:path*", + "/", + "/(de|en)/:path*", + "/docs/:lang(en|fr)*", + "/manual/:lang(en|fr)+", + "/(foo.*|bar)/:path*", + ], +}; diff --git a/tests/fixtures/middleware-matcher-auth/package.json b/tests/fixtures/middleware-matcher-auth/package.json new file mode 100644 index 0000000000..d4d192b8e7 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/package.json @@ -0,0 +1,5 @@ +{ + "name": "vinext-middleware-matcher-auth-fixture", + "private": true, + "type": "module" +} diff --git a/tests/fixtures/middleware-matcher-auth/wrangler.jsonc b/tests/fixtures/middleware-matcher-auth/wrangler.jsonc new file mode 100644 index 0000000000..ed4bb2487d --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/wrangler.jsonc @@ -0,0 +1,10 @@ +{ + "name": "vinext-middleware-matcher-auth-fixture", + "compatibility_date": "2026-04-01", + "compatibility_flags": ["nodejs_compat"], + "main": "vinext/server/fetch-handler", + "assets": { + "not_found_handling": "none", + "binding": "ASSETS", + }, +} diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts new file mode 100644 index 0000000000..228bbbf84d --- /dev/null +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -0,0 +1,10 @@ +import { matchPattern } from "../../packages/vinext/src/server/middleware-matcher.ts"; + +const nearMiss = `/${"a/".repeat(2_000)}not-end`; +for (const modifier of ["*", "+"]) { + // lgtm[js/redos] — deliberately hostile matcher executed in a timed child. + const matcher = `/:path(.*)${modifier}/end`; + if (!matchPattern(nearMiss, matcher)) { + throw new Error(`Unsafe matcher did not fail closed: ${matcher}`); + } +} diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts new file mode 100644 index 0000000000..90ec0cfa70 --- /dev/null +++ b/tests/middleware-matcher-auth.test.ts @@ -0,0 +1,290 @@ +import { execFile } from "node:child_process"; +import fs from "node:fs/promises"; +import type http from "node:http"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { promisify } from "node:util"; +import { createBuilder, type ViteDevServer } from "vite"; +import { afterAll, beforeAll, describe, expect, it } from "vite-plus/test"; +import vinext from "../packages/vinext/src/index.js"; +import { createIsolatedFixture, startFixtureServer } from "./helpers.js"; + +const FIXTURE_DIR = path.resolve(import.meta.dirname, "./fixtures/middleware-matcher-auth"); +const REDOS_CHILD = path.resolve( + import.meta.dirname, + "./fixtures/middleware-matcher-redos-child.ts", +); +const WORKSPACE_ROOT = path.resolve(import.meta.dirname, ".."); +const CLOUDFLARE_NODE_MODULES = path.resolve( + import.meta.dirname, + "./fixtures/cf-app-basic/node_modules", +); +const execFileAsync = promisify(execFile); + +const PROTECTED_PATHS = [ + "/", + "/admin/secrets", + "/dashboard/users", + "/de", + "/en/profile", + "/docs", + "/docs/en", + "/docs/en/fr", + "/manual/en", + "/manual/en/fr", + "/bar/secret", +] as const; + +async function assertAuthGuard(baseUrl: string): Promise { + const protectedResponses = await Promise.all( + PROTECTED_PATHS.map(async (pathname) => { + const response = await fetch(`${baseUrl}${pathname}`); + return { + pathname, + status: response.status, + guard: response.headers.get("x-auth-guard"), + body: await response.text(), + }; + }), + ); + expect(protectedResponses.map(({ pathname, status }) => ({ pathname, status }))).toEqual( + PROTECTED_PATHS.map((pathname) => ({ pathname, status: 403 })), + ); + for (const { pathname, guard, body } of protectedResponses) { + expect(guard, pathname).toBe("blocked"); + expect(body, pathname).toBe("blocked by middleware"); + } + + const publicResponse = await fetch(`${baseUrl}/public`); + const publicBody = await publicResponse.text(); + expect(publicResponse.status, publicBody).toBe(200); + expect(publicResponse.headers.get("x-auth-guard")).toBeNull(); + expect(publicBody).toContain("public page"); + + const constrainedMiss = await fetch(`${baseUrl}/manual/de`); + const constrainedMissBody = await constrainedMiss.text(); + expect(constrainedMiss.status, constrainedMissBody).toBe(200); + expect(constrainedMiss.headers.get("x-auth-guard")).toBeNull(); + expect(constrainedMissBody).toContain("manual secret"); +} + +async function closeHttpServer(server: http.Server | undefined): Promise { + if (!server) return; + await new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }); +} + +async function writeMatcherFixture(root: string, matcher: string): Promise { + await fs.writeFile( + path.join(root, "middleware.ts"), + `export function middleware() { return new Response("blocked", { status: 403 }) } +export const config = { matcher: ${JSON.stringify(matcher)} } +`, + ); +} + +async function findTsxCli(): Promise { + const pnpmStore = path.join(WORKSPACE_ROOT, "node_modules/.pnpm"); + const entry = (await fs.readdir(pnpmStore)).find((name) => name.startsWith("tsx@")); + if (!entry) throw new Error("tsx is not installed in the workspace dependency store"); + return path.join(pnpmStore, entry, "node_modules/tsx/dist/cli.mjs"); +} + +describe("valid middleware matcher auth guards", () => { + describe("development server", () => { + let root = ""; + let server: ViteDevServer | undefined; + let baseUrl = ""; + + beforeAll(async () => { + root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-dev-", + (source) => path.basename(source) !== "wrangler.jsonc", + ); + ({ server, baseUrl } = await startFixtureServer(root)); + }, 30_000); + + afterAll(async () => { + await server?.close(); + if (root) await fs.rm(root, { recursive: true, force: true }); + }); + + it("blocks every path selected by group and constrained-repeat matchers", async () => { + await assertAuthGuard(baseUrl); + }); + }); + + describe("built Node production server", () => { + let root = ""; + let server: http.Server | undefined; + let baseUrl = ""; + + beforeAll(async () => { + root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-node-", + (source) => path.basename(source) !== "wrangler.jsonc", + ); + const builder = await createBuilder({ + root, + configFile: false, + plugins: [vinext({ appDir: root })], + logLevel: "silent", + }); + await builder.buildApp(); + + const { startProdServer } = await import("../packages/vinext/src/server/prod-server.js"); + const started = await startProdServer({ + port: 0, + host: "127.0.0.1", + outDir: path.join(root, "dist"), + noCompression: true, + }); + server = started.server; + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Middleware matcher Node fixture did not bind to a port"); + } + baseUrl = `http://127.0.0.1:${address.port}`; + }, 120_000); + + afterAll(async () => { + await closeHttpServer(server); + if (root) await fs.rm(root, { recursive: true, force: true }); + }); + + it("blocks every path selected by group and constrained-repeat matchers", async () => { + await assertAuthGuard(baseUrl); + }); + }); + + describe("built Cloudflare Worker", () => { + let root = ""; + let worker: { url: Promise; dispose(): Promise } | undefined; + let baseUrl = ""; + + beforeAll(async () => { + root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-worker-", + undefined, + CLOUDFLARE_NODE_MODULES, + ); + const cloudflarePluginPath = path.join( + root, + "node_modules/@cloudflare/vite-plugin/dist/index.mjs", + ); + const { cloudflare } = (await import(pathToFileURL(cloudflarePluginPath).href)) as { + cloudflare: (options: { + viteEnvironment: { name: string; childEnvironments: string[] }; + }) => import("vite").Plugin; + }; + const builder = await createBuilder({ + root, + configFile: false, + plugins: [ + vinext({ appDir: root }), + cloudflare({ viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] } }), + ], + logLevel: "silent", + }); + await builder.buildApp(); + + const wranglerPath = path.join(root, "node_modules/wrangler/wrangler-dist/cli.js"); + const wrangler = (await import(pathToFileURL(wranglerPath).href)) as { + unstable_startWorker(options: { + config: string; + dev: { + remote: false; + persist: false; + logLevel: "none"; + watch: false; + server: { port: 0 }; + }; + }): Promise<{ url: Promise; dispose(): Promise }>; + }; + worker = await wrangler.unstable_startWorker({ + config: path.join(root, "dist/server/wrangler.json"), + dev: { + remote: false, + persist: false, + logLevel: "none", + watch: false, + server: { port: 0 }, + }, + }); + await worker.url; + baseUrl = (await worker.url).origin; + }, 180_000); + + afterAll(async () => { + await worker?.dispose(); + if (root) await fs.rm(root, { recursive: true, force: true }); + }); + + it("blocks every path selected by group and constrained-repeat matchers", async () => { + await assertAuthGuard(baseUrl); + }); + }); +}); + +describe("unsafe middleware matcher rejection", () => { + it.each([ + ["/:path(.*)*/end", /may match an empty value or path delimiter/], + ["/:path(.*)+/end", /may match an empty value or path delimiter/], + ["/:path((?:a+)+)", /contains nested repetition/], + ] as const)( + "rejects unsafe matcher %s during the build config phase", + async (matcher, reason) => { + const root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-unsafe-build-", + (source) => path.basename(source) !== "wrangler.jsonc", + ); + try { + await writeMatcherFixture(root, matcher); + await expect( + createBuilder({ + root, + configFile: false, + plugins: [vinext({ appDir: root })], + logLevel: "silent", + }), + ).rejects.toThrow(reason); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }, + ); + + it("rejects an ambiguous constrained repeat during dev config processing", async () => { + const root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-unsafe-dev-", + (source) => path.basename(source) !== "wrangler.jsonc", + ); + try { + await writeMatcherFixture(root, "/:path(.*)*/end"); + await expect(startFixtureServer(root)).rejects.toThrow( + /Invalid middleware matcher.*may match an empty value or path delimiter/, + ); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("fails closed on ambiguous repeats without catastrophic backtracking", async () => { + const tsxCli = await findTsxCli(); + await expect( + execFileAsync(process.execPath, [tsxCli, REDOS_CHILD], { + cwd: WORKSPACE_ROOT, + timeout: 5_000, + }), + ).resolves.toMatchObject({ + stderr: expect.stringContaining("Middleware will run for all paths"), + stdout: "", + }); + }); +}); diff --git a/tests/shims.test.ts b/tests/shims.test.ts index e6dfbb6bf8..a0c6f4697a 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vite-plus/test"; import { spawnSync } from "node:child_process"; import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { createRequire } from "node:module"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -7373,6 +7374,54 @@ describe("middleware/proxy export validation", () => { // matchPattern / matchesMiddleware unit tests describe("middleware matcher patterns", () => { + it("matches Next.js 16.2.7 path-to-regexp tokens and compiled regexes", async () => { + const { compileMiddlewareMatcherPattern } = + await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); + const { parseMiddlewarePath } = + await import("../packages/vinext/src/server/middleware-path-to-regexp.js"); + type Token = ReturnType[number]; + const nextPathToRegexp = createRequire(import.meta.url)( + "next/dist/compiled/path-to-regexp", + ) as { + parse(source: string): Token[]; + tokensToRegexp(tokens: Token[]): RegExp; + }; + const patterns = [ + "/foo\\:bar", + "/(foo.*|bar)/:path*", + "/foo/:id(a\\)b)?", + "/:id((?:foo|bar)-\\d+)?", + "/docs/:lang(en|fr)*", + "/cdn/:path*(api|static)", + "/foo-:path*", + ]; + + for (const pattern of patterns) { + const nextTokens = nextPathToRegexp.parse(pattern); + expect(parseMiddlewarePath(pattern), pattern).toEqual(nextTokens); + + let nextRegexp: RegExp; + try { + nextRegexp = nextPathToRegexp.tokensToRegexp(nextTokens); + } catch { + nextRegexp = nextPathToRegexp.tokensToRegexp( + nextTokens.map((token) => + typeof token === "object" && + (token.modifier === "*" || token.modifier === "+") && + token.prefix === "" && + token.suffix === "" + ? { ...token, prefix: "/" } + : token, + ), + ); + } + + const compiled = compileMiddlewareMatcherPattern(pattern); + expect(compiled.regexp?.source, pattern).toBe(nextRegexp.source); + expect(compiled.regexp?.flags, pattern).toBe(nextRegexp.flags); + } + }); + it("matchPattern: exact path match", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); expect(matchPattern("/about", "/about")).toBe(true); @@ -7409,6 +7458,38 @@ describe("middleware matcher patterns", () => { expect(matchPattern("/de/about", "/:locale(en|es|fr)?/about")).toBe(false); }); + // Ported from Next.js path-to-regexp matcher semantics: + // packages/next/src/lib/try-to-parse-path.ts + // https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/try-to-parse-path.ts + it("matchPattern: unnamed regex groups compose with named params", async () => { + const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); + const pattern = "/(admin|dashboard)/:path*"; + + expect(matchPattern("/admin", pattern)).toBe(true); + expect(matchPattern("/admin/secrets", pattern)).toBe(true); + expect(matchPattern("/dashboard/users/active", pattern)).toBe(true); + expect(matchPattern("/public", pattern)).toBe(false); + }); + + // path-to-regexp applies a modifier after a constraint to the entire + // slash-prefixed param and permits repeated constraint-matching segments. + it("matchPattern: constrained params repeat across path segments", async () => { + const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); + + const zeroOrMore = "/docs/:lang(en|fr)*"; + expect(matchPattern("/docs", zeroOrMore)).toBe(true); + expect(matchPattern("/docs/en", zeroOrMore)).toBe(true); + expect(matchPattern("/docs/en/fr", zeroOrMore)).toBe(true); + expect(matchPattern("/docs/de", zeroOrMore)).toBe(false); + expect(matchPattern("/docs/en/de", zeroOrMore)).toBe(false); + + const oneOrMore = "/docs/:lang(en|fr)+"; + expect(matchPattern("/docs", oneOrMore)).toBe(false); + expect(matchPattern("/docs/en", oneOrMore)).toBe(true); + expect(matchPattern("/docs/en/fr", oneOrMore)).toBe(true); + expect(matchPattern("/docs/de", oneOrMore)).toBe(false); + }); + it("matchPattern: wildcard (:path*) matches zero or more segments", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); expect(matchPattern("/dashboard", "/dashboard/:path*")).toBe(true); @@ -7417,21 +7498,48 @@ describe("middleware matcher patterns", () => { expect(matchPattern("/other", "/dashboard/:path*")).toBe(false); }); - it("matchPattern: :param*(constraint) and :param+(constraint)", async () => { + it("matchPattern: a group after :param* is a separate suffix token", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); - // /:path*(api|static) — optional segment constrained to api or static - expect(matchPattern("/cdn", "/cdn/:path*(api|static)")).toBe(true); - expect(matchPattern("/cdn/api", "/cdn/:path*(api|static)")).toBe(true); - expect(matchPattern("/cdn/static", "/cdn/:path*(api|static)")).toBe(true); + // path-to-regexp parses this as a repeated `path` param followed by an + // unnamed `(api|static)` suffix. A constrained repeat is written with the + // modifier after the group: `:path(api|static)*`. + expect(matchPattern("/cdn", "/cdn/:path*(api|static)")).toBe(false); + expect(matchPattern("/cdnapi", "/cdn/:path*(api|static)")).toBe(true); + expect(matchPattern("/cdn/fooapi", "/cdn/:path*(api|static)")).toBe(true); + expect(matchPattern("/cdn/api", "/cdn/:path*(api|static)")).toBe(false); expect(matchPattern("/cdn/other", "/cdn/:path*(api|static)")).toBe(false); - // /:path+(api|static) — required segment constrained + // `+` requires the path token, so the adjacent zero-segment form differs. expect(matchPattern("/cdn", "/cdn/:path+(api|static)")).toBe(false); - expect(matchPattern("/cdn/api", "/cdn/:path+(api|static)")).toBe(true); - expect(matchPattern("/cdn/static", "/cdn/:path+(api|static)")).toBe(true); + expect(matchPattern("/cdnapi", "/cdn/:path+(api|static)")).toBe(false); + expect(matchPattern("/cdn/fooapi", "/cdn/:path+(api|static)")).toBe(true); + expect(matchPattern("/cdn/api", "/cdn/:path+(api|static)")).toBe(false); expect(matchPattern("/cdn/other", "/cdn/:path+(api|static)")).toBe(false); }); + it("matchPattern: preserves escaped and nested path-to-regexp syntax", async () => { + const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); + + expect(matchPattern("/foo:bar", "/foo\\:bar")).toBe(true); + expect(matchPattern("/foo/bar", "/foo\\:bar")).toBe(false); + + const mixed = "/(foo.*|bar)/:path*"; + expect(matchPattern("/foo-value/child", mixed)).toBe(true); + expect(matchPattern("/bar", mixed)).toBe(true); + expect(matchPattern("/baz/child", mixed)).toBe(false); + + const escapedClose = "/foo/:id(a\\)b)?"; + expect(matchPattern("/foo", escapedClose)).toBe(true); + expect(matchPattern("/foo/a)b", escapedClose)).toBe(true); + expect(matchPattern("/foo/ab", escapedClose)).toBe(false); + + const nested = "/:id((?:foo|bar)-\\d+)?"; + expect(matchPattern("/", nested)).toBe(true); + expect(matchPattern("/foo-123", nested)).toBe(true); + expect(matchPattern("/bar-9", nested)).toBe(true); + expect(matchPattern("/baz-9", nested)).toBe(false); + }); + it("matchPattern: one-or-more (:path+) requires at least one segment", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); expect(matchPattern("/api/users", "/api/:path+")).toBe(true); @@ -7680,17 +7788,17 @@ describe("middleware matcher patterns", () => { it("matchPattern: fails closed for pathological ReDoS patterns", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); - // Pathological pattern: (a+)+ causes catastrophic backtracking + // Pathological constraint: (?:a+)+ causes catastrophic backtracking. // matchPattern must avoid evaluating it and run middleware instead of // silently bypassing a potentially security-sensitive request guard. // lgtm[js/redos] — deliberate pathological regex to test safeRegExp guard - expect(matchPattern("/aaaaaaaaaaaaaaaaaaaac", "(a+)+b")).toBe(true); + expect(matchPattern("/aaaaaaaaaaaaaaaaaaaac", "/:path((?:a+)+)")).toBe(true); }); - it("matchPattern: does not run globally for malformed regex syntax", async () => { + it("matchPattern: fails closed for malformed matcher syntax", async () => { const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); - expect(matchPattern("/public", "/admin(")).toBe(false); + expect(matchPattern("/public", "/admin(")).toBe(true); expect(matchPattern("/admin(", "/admin(")).toBe(true); }); }); From 6ddfcf94e4cdb1b602eb6309c6a3f36639acf812 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 10 Jul 2026 13:50:26 +0100 Subject: [PATCH 2/9] fix(middleware): harden matcher validation --- packages/vinext/src/build/report.ts | 13 +- packages/vinext/src/config/config-matchers.ts | 6 + packages/vinext/src/index.ts | 8 +- .../src/server/middleware-matcher-pattern.ts | 221 +++++++++++++++++- .../vinext/src/server/middleware-matcher.ts | 52 ++--- .../app/report.json/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 1 + .../middleware-matcher-redos-child.ts | 7 + tests/middleware-matcher-auth.test.ts | 39 ++++ tests/shims.test.ts | 17 +- 10 files changed, 318 insertions(+), 49 deletions(-) create mode 100644 tests/fixtures/middleware-matcher-auth/app/report.json/page.tsx diff --git a/packages/vinext/src/build/report.ts b/packages/vinext/src/build/report.ts index c64dc75398..db187927f8 100644 --- a/packages/vinext/src/build/report.ts +++ b/packages/vinext/src/build/report.ts @@ -245,6 +245,17 @@ function extractStringFromConstInitializer(initializer: Expression | null): stri export function extractMiddlewareMatcherConfig( filePath: string, ): StaticMiddlewareMatcher | undefined { + const value = extractMiddlewareMatcherConfigValue(filePath); + return isStaticMiddlewareMatcher(value) ? value : undefined; +} + +/** + * Extract the statically analyzable `config.matcher` value without first + * narrowing it to vinext's runtime matcher type. Build validation needs the + * raw value so malformed matcher objects are rejected instead of disappearing + * as though no matcher had been configured. + */ +export function extractMiddlewareMatcherConfigValue(filePath: string): unknown { let code: string; try { code = fs.readFileSync(filePath, "utf8"); @@ -261,7 +272,7 @@ export function extractMiddlewareMatcherConfig( if (!matcherExpression) return undefined; const value = extractStaticJsonValue(matcherExpression); - return isStaticMiddlewareMatcher(value) ? value : undefined; + return value === UNSUPPORTED_STATIC_VALUE ? undefined : value; } function objectPropertyValue(object: ObjectExpression, key: string): Expression | null { diff --git a/packages/vinext/src/config/config-matchers.ts b/packages/vinext/src/config/config-matchers.ts index a2a8b818c3..766c2b9dd1 100644 --- a/packages/vinext/src/config/config-matchers.ts +++ b/packages/vinext/src/config/config-matchers.ts @@ -346,6 +346,12 @@ export function isSafeRegex(pattern: string): boolean { } if (ch === "?") { + // Group prefixes such as (?:...), (?=...), (?!...), and (?<=...) are + // syntax markers, not optional quantifiers. + if (i > 0 && pattern[i - 1] === "(" && /[:=!<]/.test(pattern[i + 1] ?? "")) { + i++; + continue; + } // '?' after +, *, ?, or } is a non-greedy modifier, not a quantifier const prev = i > 0 ? pattern[i - 1] : ""; if (prev !== "+" && prev !== "*" && prev !== "?" && prev !== "}") { diff --git a/packages/vinext/src/index.ts b/packages/vinext/src/index.ts index 443ac40aa3..7bc0915dc2 100644 --- a/packages/vinext/src/index.ts +++ b/packages/vinext/src/index.ts @@ -68,7 +68,11 @@ import { collectRouteClassificationManifest, type RouteClassificationManifest, } from "./build/route-classification-manifest.js"; -import { extractMiddlewareMatcherConfig, hasExportedName } from "./build/report.js"; +import { + extractMiddlewareMatcherConfig, + extractMiddlewareMatcherConfigValue, + hasExportedName, +} from "./build/report.js"; import { planRouteClassificationInjection } from "./build/route-classification-injector.js"; import { normalizePathnameForRouteMatchStrict } from "./routing/utils.js"; import { @@ -2011,7 +2015,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { : root; middlewarePath = findMiddlewareFile(root, fileMatcher, middlewareConventionDir); if (middlewarePath) { - const staticMatcher = extractMiddlewareMatcherConfig(middlewarePath); + const staticMatcher = extractMiddlewareMatcherConfigValue(middlewarePath); if (staticMatcher !== undefined) { validateMiddlewareMatcherPatterns(staticMatcher); } diff --git a/packages/vinext/src/server/middleware-matcher-pattern.ts b/packages/vinext/src/server/middleware-matcher-pattern.ts index 130524f393..9559fdea24 100644 --- a/packages/vinext/src/server/middleware-matcher-pattern.ts +++ b/packages/vinext/src/server/middleware-matcher-pattern.ts @@ -1,4 +1,5 @@ import { isSafeRegex } from "../config/config-matchers.js"; +import type { HasCondition } from "../config/next-config.js"; import { middlewarePathTokensToRegExp, normalizeMiddlewarePathTokens, @@ -11,6 +12,90 @@ export type CompiledMiddlewareMatcherPattern = | { regexp: RegExp; error?: never } | { regexp?: never; error: string; kind: "invalid" | "unsafe" }; +export type MiddlewareMatcherObject = { + source: string; + locale?: false; + has?: HasCondition[]; + missing?: HasCondition[]; +}; + +const MATCHER_OBJECT_KEYS = new Set(["source", "locale", "has", "missing"]); +const CONDITION_TYPES_WITH_KEY = new Set(["header", "query", "cookie"]); + +function invalidConditionReason(value: unknown): string | null { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return "has and missing entries must be objects"; + } + + const condition = value as Record; + const type = condition.type; + if (typeof type === "string" && CONDITION_TYPES_WITH_KEY.has(type)) { + for (const key of Object.keys(condition)) { + if (key !== "type" && key !== "key" && key !== "value") { + return `condition contains unsupported field "${key}"`; + } + } + if (typeof condition.key !== "string") { + return `condition type "${type}" requires a string key`; + } + if (condition.value !== undefined && typeof condition.value !== "string") { + return `condition type "${type}" requires value to be a string`; + } + return null; + } + + if (type === "host") { + for (const key of Object.keys(condition)) { + if (key !== "type" && key !== "value") { + return `host condition contains unsupported field "${key}"`; + } + } + return typeof condition.value === "string" ? null : "host condition requires a string value"; + } + + return "condition type must be header, query, cookie, or host"; +} + +function invalidConditionsReason(value: unknown, field: "has" | "missing"): string | null { + if (value === undefined) return null; + if (!Array.isArray(value)) return `${field} must be an array`; + for (const condition of value) { + const reason = invalidConditionReason(condition); + if (reason) return `${field} ${reason}`; + } + return null; +} + +function matcherObjectSource(value: unknown): { source?: string; error?: string } { + if (!value || typeof value !== "object" || Array.isArray(value)) { + return { error: "matcher entries must be strings or objects" }; + } + + const matcher = value as Record; + for (const key of Object.keys(matcher)) { + if (!MATCHER_OBJECT_KEYS.has(key)) { + return { error: `matcher object contains unsupported field "${key}"` }; + } + } + if (typeof matcher.source !== "string") { + return { error: "matcher object requires a string source" }; + } + if (matcher.locale !== undefined && matcher.locale !== false) { + return { error: "matcher object locale must be false when provided" }; + } + const invalidHas = invalidConditionsReason(matcher.has, "has"); + if (invalidHas) return { error: invalidHas }; + const invalidMissing = invalidConditionsReason(matcher.missing, "missing"); + if (invalidMissing) return { error: invalidMissing }; + return { source: matcher.source }; +} + +export function isValidMiddlewareMatcherObjectConfig( + value: unknown, +): value is MiddlewareMatcherObject { + return matcherObjectSource(value).error === undefined; +} + function patternMatches(pattern: string, value: string): boolean { try { return new RegExp(`^(?:${pattern})$`).test(value); @@ -19,10 +104,135 @@ function patternMatches(pattern: string, value: string): boolean { } } +function atomsOverlap(left: string, right: string): boolean { + let leftRegexp: RegExp; + let rightRegexp: RegExp; + try { + leftRegexp = new RegExp(`^(?:${left})$`); + rightRegexp = new RegExp(`^(?:${right})$`); + } catch { + return true; + } + + // Middleware paths are URL text. The ASCII URL alphabet is sufficient to + // prove overlap for the common literal, dot, shorthand, and class atoms; + // include one non-ASCII code point for broad Unicode classes. + for (let code = 0x20; code <= 0x7e; code++) { + const character = String.fromCharCode(code); + if (leftRegexp.test(character) && rightRegexp.test(character)) return true; + } + return leftRegexp.test("é") && rightRegexp.test("é"); +} + +function groupCanMatchEmpty(group: string): boolean { + if (/^\?(?:[=!]|<[=!])/.test(group)) return true; + const body = group.startsWith("?:") ? group.slice(2) : group; + return patternMatches(body, ""); +} + +function hasOverlappingSequentialRepetition(pattern: string): boolean { + const repeatedAtDepth: string[][] = [[]]; + const groupStarts: number[] = []; + let depth = 0; + + for (let index = 0; index < pattern.length; index++) { + const character = pattern[index]; + if (character === "(") { + groupStarts.push(index); + depth++; + repeatedAtDepth[depth] = []; + if (pattern[index + 1] === "?") { + // Skip the non-capturing/lookaround marker. Its punctuation is not a + // consuming atom, and the existing validator handles quantified + // lookaround groups conservatively. + index++; + if (pattern[index + 1] === "<" && /[=!]/.test(pattern[index + 2] ?? "")) index += 2; + else if (/[=:!]/.test(pattern[index + 1] ?? "")) index++; + } + continue; + } + if (character === ")") { + const groupStart = groupStarts.pop(); + repeatedAtDepth[depth] = []; + depth = Math.max(0, depth - 1); + if (groupStart !== undefined) { + const modifier = pattern[index + 1]; + const optionalModifier = + modifier === "*" || + modifier === "?" || + (modifier === "{" && /^\{0(?:,\d*)?\}/.test(pattern.slice(index + 1))); + if (!optionalModifier && !groupCanMatchEmpty(pattern.slice(groupStart + 1, index))) { + repeatedAtDepth[depth] = []; + } + } + continue; + } + if (character === "|") { + repeatedAtDepth[depth] = []; + continue; + } + if (character === "^" || character === "$") continue; + if (character === "*" || character === "+" || character === "?" || character === "{") { + continue; + } + + let atom = character; + if (character === "\\") { + if (index + 1 >= pattern.length) return true; + atom += pattern[++index]; + } else if (character === "[") { + let classEnd = index + 1; + if (pattern[classEnd] === "^") classEnd++; + while (classEnd < pattern.length && pattern[classEnd] !== "]") { + if (pattern[classEnd] === "\\") classEnd++; + classEnd++; + } + if (classEnd >= pattern.length) return true; + atom = pattern.slice(index, classEnd + 1); + index = classEnd; + } else if (character === ".") { + atom = "."; + } + + const quantifierStart = index + 1; + const quantifier = pattern[quantifierStart]; + let unbounded = quantifier === "*" || quantifier === "+"; + let canBeEmpty = quantifier === "*"; + let quantifierEnd = quantifierStart; + if (quantifier === "{") { + const match = /^\{(\d+),\}/.exec(pattern.slice(quantifierStart)); + if (match) { + unbounded = true; + canBeEmpty = Number(match[1]) === 0; + quantifierEnd += match[0].length - 1; + } + } + + if (!unbounded) { + // An optional atom does not separate the repetitions on either side; + // every other non-repeated atom is a mandatory separator. + if (quantifier !== "?") repeatedAtDepth[depth] = []; + if (quantifier === "?") index = quantifierStart; + continue; + } + + if (repeatedAtDepth[depth].some((previous) => atomsOverlap(previous, atom))) { + return true; + } + repeatedAtDepth[depth] = canBeEmpty ? [...repeatedAtDepth[depth], atom] : [atom]; + index = quantifierEnd; + } + + return false; +} + function unsafeTokenReason(token: MiddlewarePathKey): string | null { if (!isSafeRegex(token.pattern)) { return `parameter "${token.name}" contains nested repetition`; } + if (hasOverlappingSequentialRepetition(token.pattern)) { + return `parameter "${token.name}" contains overlapping sequential repetition`; + } if (token.modifier !== "*" && token.modifier !== "+") return null; @@ -94,11 +304,16 @@ export function validateMiddlewareMatcherPatterns(value: unknown): void { } else if (Array.isArray(value)) { for (const matcher of value) { if (typeof matcher === "string") sources.push(matcher); - else if (matcher && typeof matcher === "object" && "source" in matcher) { - const source = Reflect.get(matcher, "source"); - if (typeof source === "string") sources.push(source); + else { + const result = matcherObjectSource(matcher); + if (result.error) throw new Error(`Invalid middleware matcher config: ${result.error}.`); + sources.push(result.source!); } } + } else { + throw new Error( + "Invalid middleware matcher config: matcher must be a string or an array of strings or objects.", + ); } for (const source of sources) { diff --git a/packages/vinext/src/server/middleware-matcher.ts b/packages/vinext/src/server/middleware-matcher.ts index 0a1b05c21d..d326745933 100644 --- a/packages/vinext/src/server/middleware-matcher.ts +++ b/packages/vinext/src/server/middleware-matcher.ts @@ -3,16 +3,13 @@ import { requestContextFromRequest, type RequestContext, } from "../config/config-matchers.js"; -import type { HasCondition, NextI18nConfig } from "../config/next-config.js"; +import type { NextI18nConfig } from "../config/next-config.js"; import { removeTrailingSlash } from "../utils/base-path.js"; -import { compileMiddlewareMatcherPattern } from "./middleware-matcher-pattern.js"; - -type MiddlewareMatcherObject = { - source: string; - locale?: false; - has?: HasCondition[]; - missing?: HasCondition[]; -}; +import { + compileMiddlewareMatcherPattern, + isValidMiddlewareMatcherObjectConfig, + type MiddlewareMatcherObject, +} from "./middleware-matcher-pattern.js"; export type MatcherConfig = string | Array; @@ -42,7 +39,7 @@ export function matchesMiddleware( return matchMatcherPattern(pathname, matcher, i18nConfig); } if (!Array.isArray(matcher)) { - return false; + return true; } const requestContext = request @@ -57,40 +54,21 @@ export function matchesMiddleware( continue; } - if (isValidMiddlewareMatcherObject(m)) { - if (!matchObjectMatcher(pathname, m, i18nConfig)) { - continue; - } - - if (!checkHasConditions(m.has, m.missing, requestContext)) { - continue; - } - + if (!isValidMiddlewareMatcherObjectConfig(m)) { return true; } - } - - return false; -} - -function isValidMiddlewareMatcherObject(value: unknown): value is MiddlewareMatcherObject { - if (!value || typeof value !== "object" || Array.isArray(value)) return false; - - if (!("source" in value) || typeof value.source !== "string") return false; + if (!matchObjectMatcher(pathname, m, i18nConfig)) { + continue; + } - for (const key of Object.keys(value)) { - if (key !== "source" && key !== "locale" && key !== "has" && key !== "missing") { - return false; + if (!checkHasConditions(m.has, m.missing, requestContext)) { + continue; } - } - if ("locale" in value && value.locale !== undefined && value.locale !== false) return false; - if ("has" in value && value.has !== undefined && !Array.isArray(value.has)) return false; - if ("missing" in value && value.missing !== undefined && !Array.isArray(value.missing)) { - return false; + return true; } - return true; + return false; } function matchMatcherPattern( diff --git a/tests/fixtures/middleware-matcher-auth/app/report.json/page.tsx b/tests/fixtures/middleware-matcher-auth/app/report.json/page.tsx new file mode 100644 index 0000000000..8247f61f61 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/report.json/page.tsx @@ -0,0 +1,3 @@ +export default function ReportPage() { + return
report secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts index a4f2356ad2..7186553d0a 100644 --- a/tests/fixtures/middleware-matcher-auth/middleware.ts +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -15,5 +15,6 @@ export const config = { "/docs/:lang(en|fr)*", "/manual/:lang(en|fr)+", "/(foo.*|bar)/:path*", + "/report{.:ext}", ], }; diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index 228bbbf84d..0f57733c7e 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -8,3 +8,10 @@ for (const modifier of ["*", "+"]) { throw new Error(`Unsafe matcher did not fail closed: ${matcher}`); } } + +// Sequential repetitions that can consume the same text can also produce +// catastrophic backtracking without a quantifier directly wrapping a group. +const overlappingRepetition = "/:path(a+.*a+)"; +if (!matchPattern(`/${"a".repeat(3_000)}b`, overlappingRepetition)) { + throw new Error(`Unsafe matcher did not fail closed: ${overlappingRepetition}`); +} diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 90ec0cfa70..3ebcf24550 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -33,6 +33,7 @@ const PROTECTED_PATHS = [ "/manual/en", "/manual/en/fr", "/bar/secret", + "/report.json", ] as const; async function assertAuthGuard(baseUrl: string): Promise { @@ -235,6 +236,8 @@ describe("unsafe middleware matcher rejection", () => { ["/:path(.*)*/end", /may match an empty value or path delimiter/], ["/:path(.*)+/end", /may match an empty value or path delimiter/], ["/:path((?:a+)+)", /contains nested repetition/], + ["/:path(a+.*a+)", /contains overlapping sequential repetition/], + ["/:path(a+(?:b*)a+)", /contains overlapping sequential repetition/], ] as const)( "rejects unsafe matcher %s during the build config phase", async (matcher, reason) => { @@ -288,3 +291,39 @@ describe("unsafe middleware matcher rejection", () => { }); }); }); + +describe("invalid middleware matcher object auth guards", () => { + it("rejects an object matcher with an unsupported field before production build", async () => { + const root = await createIsolatedFixture( + FIXTURE_DIR, + "vinext-middleware-matcher-invalid-object-", + (source) => path.basename(source) !== "wrangler.jsonc", + ); + try { + await fs.writeFile( + path.join(root, "middleware.ts"), + `export function middleware() { + return new Response("blocked by middleware", { + status: 403, + headers: { "x-auth-guard": "blocked" }, + }) +} +export const config = { + matcher: [{ source: "/admin/:path*", typo: true }], +} +`, + ); + + await expect( + createBuilder({ + root, + configFile: false, + plugins: [vinext({ appDir: root })], + logLevel: "silent", + }), + ).rejects.toThrow(/matcher object contains unsupported field "typo"/); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/tests/shims.test.ts b/tests/shims.test.ts index a0c6f4697a..6d2ed4aade 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7394,6 +7394,10 @@ describe("middleware matcher patterns", () => { "/docs/:lang(en|fr)*", "/cdn/:path*(api|static)", "/foo-:path*", + "/foo{/:id}?", + "/files/:name{.:ext}?", + "/foo{-:bar}+", + "/x/:id(a+(?:b)a+)", ]; for (const pattern of patterns) { @@ -7725,14 +7729,14 @@ describe("middleware matcher patterns", () => { ).toBe(false); }); - it("matchesMiddleware: rejects a single object matcher config", async () => { + it("matchesMiddleware: fails closed for a single object matcher config", async () => { const { matchesMiddleware } = await import("../packages/vinext/src/server/middleware.js"); const matcher: any = { source: "/dashboard", has: [{ type: "header", key: "x-user-tier", value: "pro" }], }; - expect(matchesMiddleware("/dashboard", matcher)).toBe(false); + expect(matchesMiddleware("/dashboard", matcher)).toBe(true); expect( matchesMiddleware( "/dashboard", @@ -7741,15 +7745,15 @@ describe("middleware matcher patterns", () => { headers: { "x-user-tier": "pro" }, }), ), - ).toBe(false); + ).toBe(true); }); - it("matchesMiddleware: rejects object matchers with unsupported fields", async () => { + it("matchesMiddleware: fails closed for object matchers with unsupported fields", async () => { const { matchesMiddleware } = await import("../packages/vinext/src/server/middleware.js"); const matcher: any = [{ source: "/does-not-match", regexp: "^/dashboard(?:/.*)?$" }]; - expect(matchesMiddleware("/dashboard/settings", matcher)).toBe(false); - expect(matchesMiddleware("/about", matcher)).toBe(false); + expect(matchesMiddleware("/dashboard/settings", matcher)).toBe(true); + expect(matchesMiddleware("/about", matcher)).toBe(true); }); it("matchesMiddleware: matches default-locale and locale-prefixed paths unless locale is false", async () => { @@ -10824,6 +10828,7 @@ describe("isSafeRegex", () => { // (ab)+ is fine — no inner quantifier expect(isSafeRegex("(ab)+")).toBe(true); expect(isSafeRegex("(foo|bar)*")).toBe(true); + expect(isSafeRegex("(?:(?!\\.)[^/])+?")).toBe(true); }); it("treats escaped characters as safe", async () => { From 368df6d0e9be67e49a7be6fd56fff6f90ae0aace Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:02:10 +0100 Subject: [PATCH 3/9] fix(middleware): close matcher parity gaps --- packages/vinext/src/config/config-matchers.ts | 32 +++- .../src/server/middleware-matcher-pattern.ts | 173 ++++++++++++++++++ .../app/archive/[date]/page.tsx | 3 + .../app/conditioned/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 12 ++ .../middleware-matcher-redos-child.ts | 7 + tests/middleware-matcher-auth.test.ts | 31 ++++ tests/shims.test.ts | 91 +++++++++ 8 files changed, 342 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/middleware-matcher-auth/app/archive/[date]/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/conditioned/page.tsx diff --git a/packages/vinext/src/config/config-matchers.ts b/packages/vinext/src/config/config-matchers.ts index 766c2b9dd1..30ee496471 100644 --- a/packages/vinext/src/config/config-matchers.ts +++ b/packages/vinext/src/config/config-matchers.ts @@ -315,13 +315,21 @@ export function isSafeRegex(pattern: string): boolean { const hadQuantifier = depth > 0 && quantifierAtDepth[depth]; if (depth > 0) depth--; - // Look ahead for a quantifier on this group: +, *, {n,m} - // Note: '?' after ')' means "zero or one" which does NOT cause catastrophic - // backtracking — it only allows 2 paths (match/skip), not exponential. - // Only unbounded repetition (+, *, {n,}) on a group with inner quantifiers is dangerous. + // Only unbounded repetition of a group with inner quantifiers is + // intrinsically dangerous. A finite group repetition such as + // `(?:-\d{2}){2}` has a bounded number of paths and is safe by itself. + // Still propagate every group quantifier to the enclosing group so a + // later unbounded repetition around it is rejected. const next = pattern[i + 1]; - if (next === "+" || next === "*" || next === "{") { - if (hadQuantifier) { + let hasGroupQuantifier = next === "+" || next === "*" || next === "?"; + let hasUnboundedGroupQuantifier = next === "+" || next === "*"; + if (next === "{") { + const braceQuantifier = /^\{\d+(,\d*)?\}/.exec(pattern.slice(i + 1)); + hasGroupQuantifier = braceQuantifier !== null; + hasUnboundedGroupQuantifier = braceQuantifier?.[1]?.endsWith(",") === true; + } + if (hasGroupQuantifier) { + if (hasUnboundedGroupQuantifier && hadQuantifier) { // Nested quantifier detected: quantifier on a group that contains a quantifier return false; } @@ -625,7 +633,9 @@ function _matchConditionValue( actualValue: string, expectedValue: string | undefined, ): Record | null { - if (expectedValue === undefined) return _emptyParams(); + // Next.js treats an omitted or empty condition value as a presence check. + // Its matchHas helper also requires the actual value to be non-empty. + if (!expectedValue) return actualValue ? _emptyParams() : null; const re = _cachedConditionRegex(expectedValue); if (re) { @@ -664,9 +674,11 @@ function matchSingleCondition( return _matchConditionValue(cookieValue, condition.value); } case "query": { - const queryValue = ctx.query.get(condition.key); - if (queryValue === null) return null; - return _matchConditionValue(queryValue, condition.value); + const queryValues = ctx.query.getAll(condition.key); + if (queryValues.length === 0) return null; + // Node parses duplicate query keys as an array and Next.js matchHas + // explicitly tests its final value (`value.slice(-1)[0]`). + return _matchConditionValue(queryValues[queryValues.length - 1], condition.value); } case "host": { if (condition.value !== undefined) return _matchConditionValue(ctx.host, condition.value); diff --git a/packages/vinext/src/server/middleware-matcher-pattern.ts b/packages/vinext/src/server/middleware-matcher-pattern.ts index 9559fdea24..66e8cf6b6a 100644 --- a/packages/vinext/src/server/middleware-matcher-pattern.ts +++ b/packages/vinext/src/server/middleware-matcher-pattern.ts @@ -124,6 +124,176 @@ function atomsOverlap(left: string, right: string): boolean { return leftRegexp.test("é") && rightRegexp.test("é"); } +function findGroupEnd(pattern: string, start: number): number | null { + let depth = 0; + let inClass = false; + for (let index = start; index < pattern.length; index++) { + const character = pattern[index]; + if (character === "\\") { + index++; + continue; + } + if (character === "[") { + inClass = true; + continue; + } + if (character === "]" && inClass) { + inClass = false; + continue; + } + if (inClass) continue; + if (character === "(") depth++; + if (character === ")" && --depth === 0) return index; + } + return null; +} + +function splitTopLevelAlternatives(pattern: string): string[] { + const alternatives: string[] = []; + let start = 0; + let depth = 0; + let inClass = false; + for (let index = 0; index < pattern.length; index++) { + const character = pattern[index]; + if (character === "\\") { + index++; + continue; + } + if (character === "[") { + inClass = true; + continue; + } + if (character === "]" && inClass) { + inClass = false; + continue; + } + if (inClass) continue; + if (character === "(") depth++; + else if (character === ")") depth--; + else if (character === "|" && depth === 0) { + alternatives.push(pattern.slice(start, index)); + start = index + 1; + } + } + alternatives.push(pattern.slice(start)); + return alternatives; +} + +function firstConsumingAtoms(branch: string): string[] | null { + for (let index = 0; index < branch.length; index++) { + const character = branch[index]; + if (character === "^" || character === "$") continue; + if (character === "\\") { + return index + 1 < branch.length ? [branch.slice(index, index + 2)] : null; + } + if (character === "[") { + let end = index + 1; + if (branch[end] === "^") end++; + while (end < branch.length && branch[end] !== "]") { + if (branch[end] === "\\") end++; + end++; + } + return end < branch.length ? [branch.slice(index, end + 1)] : null; + } + if (character === ".") return ["."]; + if (character === "(") { + const end = findGroupEnd(branch, index); + if (end === null) return null; + const body = branch.slice(index + 1, end); + if (/^\?(?:[=!]|<[=!])/.test(body)) { + // Lookarounds consume no text, so the following atom owns the prefix. + index = end; + continue; + } + const groupBody = body.startsWith("?:") ? body.slice(2) : body; + const alternatives = splitTopLevelAlternatives(groupBody); + const atoms = alternatives.flatMap((alternative) => firstConsumingAtoms(alternative) ?? []); + return atoms.length === alternatives.length ? atoms : null; + } + if ("*+?{}|)".includes(character)) return null; + return [character]; + } + return null; +} + +function alternativesMaySharePrefix(pattern: string): boolean { + const alternatives = splitTopLevelAlternatives(pattern); + if (alternatives.length > 1) { + const firstAtoms = alternatives.map(firstConsumingAtoms); + for (let left = 0; left < firstAtoms.length; left++) { + for (let right = left + 1; right < firstAtoms.length; right++) { + const leftAtoms = firstAtoms[left]; + const rightAtoms = firstAtoms[right]; + // Unknown or empty branches cannot be proven prefix-disjoint, so fail + // closed when the alternation itself is repeated without a bound. + if (!leftAtoms || !rightAtoms) return true; + if (leftAtoms.some((a) => rightAtoms.some((b) => atomsOverlap(a, b)))) return true; + } + } + } + + // An alternation nested inside an unbounded repeated group can still make + // the repeated language ambiguous even when wrapped in a non-capturing + // group or preceded by shared literals. Inspect all child groups as well. + for (let index = 0; index < pattern.length; index++) { + if (pattern[index] === "\\") { + index++; + continue; + } + if (pattern[index] === "[") { + while (++index < pattern.length && pattern[index] !== "]") { + if (pattern[index] === "\\") index++; + } + continue; + } + if (pattern[index] !== "(") continue; + const end = findGroupEnd(pattern, index); + if (end === null) return true; + const body = pattern.slice(index + 1, end); + if (!/^\?(?:[=!]|<[=!])/.test(body)) { + const groupBody = body.startsWith("?:") ? body.slice(2) : body; + if (alternativesMaySharePrefix(groupBody)) return true; + } + index = end; + } + return false; +} + +function hasAmbiguousQuantifiedAlternation(pattern: string): boolean { + const groupStarts: number[] = []; + let inClass = false; + for (let index = 0; index < pattern.length; index++) { + const character = pattern[index]; + if (character === "\\") { + index++; + continue; + } + if (character === "[") { + inClass = true; + continue; + } + if (character === "]" && inClass) { + inClass = false; + continue; + } + if (inClass) continue; + if (character === "(") { + groupStarts.push(index); + continue; + } + if (character !== ")") continue; + const start = groupStarts.pop(); + if (start === undefined) return true; + const suffix = pattern.slice(index + 1); + const unbounded = suffix[0] === "+" || suffix[0] === "*" || /^\{\d+,\}/.test(suffix); + if (!unbounded) continue; + const body = pattern.slice(start + 1, index); + const groupBody = body.startsWith("?:") ? body.slice(2) : body; + if (alternativesMaySharePrefix(groupBody)) return true; + } + return groupStarts.length > 0; +} + function groupCanMatchEmpty(group: string): boolean { if (/^\?(?:[=!]|<[=!])/.test(group)) return true; const body = group.startsWith("?:") ? group.slice(2) : group; @@ -230,6 +400,9 @@ function unsafeTokenReason(token: MiddlewarePathKey): string | null { if (!isSafeRegex(token.pattern)) { return `parameter "${token.name}" contains nested repetition`; } + if (hasAmbiguousQuantifiedAlternation(token.pattern)) { + return `parameter "${token.name}" contains ambiguous alternatives under unbounded repetition`; + } if (hasOverlappingSequentialRepetition(token.pattern)) { return `parameter "${token.name}" contains overlapping sequential repetition`; } diff --git a/tests/fixtures/middleware-matcher-auth/app/archive/[date]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/archive/[date]/page.tsx new file mode 100644 index 0000000000..f1cf1053c2 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/archive/[date]/page.tsx @@ -0,0 +1,3 @@ +export default function ArchivePage() { + return
archive secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/conditioned/page.tsx b/tests/fixtures/middleware-matcher-auth/app/conditioned/page.tsx new file mode 100644 index 0000000000..c6adaa3dab --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/conditioned/page.tsx @@ -0,0 +1,3 @@ +export default function ConditionedPage() { + return
conditioned page
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts index 7186553d0a..8166b7a2eb 100644 --- a/tests/fixtures/middleware-matcher-auth/middleware.ts +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -16,5 +16,17 @@ export const config = { "/manual/:lang(en|fr)+", "/(foo.*|bar)/:path*", "/report{.:ext}", + "/archive/:date(\\d{4}(?:-\\d{2}){2})", + { + source: "/conditioned", + has: [ + { type: "query", key: "role", value: "admin" }, + { type: "query", key: "present", value: "" }, + { type: "header", key: "x-present", value: "" }, + { type: "cookie", key: "session", value: "" }, + { type: "host", value: "" }, + ], + missing: [{ type: "query", key: "blocked", value: "1" }], + }, ], }; diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index 0f57733c7e..b3c108b1b2 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -15,3 +15,10 @@ const overlappingRepetition = "/:path(a+.*a+)"; if (!matchPattern(`/${"a".repeat(3_000)}b`, overlappingRepetition)) { throw new Error(`Unsafe matcher did not fail closed: ${overlappingRepetition}`); } + +// Alternations where one branch prefixes another have exponentially many +// partitions under an unbounded group repetition. +const ambiguousAlternative = "/:path((?:a|aa)+)"; +if (!matchPattern(`/${"a".repeat(3_000)}b`, ambiguousAlternative)) { + throw new Error(`Unsafe matcher did not fail closed: ${ambiguousAlternative}`); +} diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 3ebcf24550..1d6286ffdd 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -34,6 +34,7 @@ const PROTECTED_PATHS = [ "/manual/en/fr", "/bar/secret", "/report.json", + "/archive/2024-07-10", ] as const; async function assertAuthGuard(baseUrl: string): Promise { @@ -67,6 +68,35 @@ async function assertAuthGuard(baseUrl: string): Promise { expect(constrainedMiss.status, constrainedMissBody).toBe(200); expect(constrainedMiss.headers.get("x-auth-guard")).toBeNull(); expect(constrainedMissBody).toContain("manual secret"); + + const conditionedHeaders = { + "x-present": "yes", + cookie: "session=active", + }; + const conditioned = await fetch( + `${baseUrl}/conditioned?role=guest&role=admin&present=yes&blocked=1&blocked=0`, + { headers: conditionedHeaders }, + ); + expect(conditioned.status, await conditioned.text()).toBe(403); + expect(conditioned.headers.get("x-auth-guard")).toBe("blocked"); + + const wrongLastHas = await fetch( + `${baseUrl}/conditioned?role=admin&role=guest&present=yes&blocked=1&blocked=0`, + { headers: conditionedHeaders }, + ); + const wrongLastHasBody = await wrongLastHas.text(); + expect(wrongLastHas.status, wrongLastHasBody).toBe(200); + expect(wrongLastHas.headers.get("x-auth-guard")).toBeNull(); + expect(wrongLastHasBody).toContain("conditioned page"); + + const wrongLastMissing = await fetch( + `${baseUrl}/conditioned?role=guest&role=admin&present=yes&blocked=0&blocked=1`, + { headers: conditionedHeaders }, + ); + const wrongLastMissingBody = await wrongLastMissing.text(); + expect(wrongLastMissing.status, wrongLastMissingBody).toBe(200); + expect(wrongLastMissing.headers.get("x-auth-guard")).toBeNull(); + expect(wrongLastMissingBody).toContain("conditioned page"); } async function closeHttpServer(server: http.Server | undefined): Promise { @@ -236,6 +266,7 @@ describe("unsafe middleware matcher rejection", () => { ["/:path(.*)*/end", /may match an empty value or path delimiter/], ["/:path(.*)+/end", /may match an empty value or path delimiter/], ["/:path((?:a+)+)", /contains nested repetition/], + ["/:path((?:a|aa)+)", /contains ambiguous alternatives under unbounded repetition/], ["/:path(a+.*a+)", /contains overlapping sequential repetition/], ["/:path(a+(?:b*)a+)", /contains overlapping sequential repetition/], ] as const)( diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 6d2ed4aade..e865d85f5d 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7398,6 +7398,7 @@ describe("middleware matcher patterns", () => { "/files/:name{.:ext}?", "/foo{-:bar}+", "/x/:id(a+(?:b)a+)", + "/archive/:date(\\d{4}(?:-\\d{2}){2})", ]; for (const pattern of patterns) { @@ -7462,6 +7463,31 @@ describe("middleware matcher patterns", () => { expect(matchPattern("/de/about", "/:locale(en|es|fr)?/about")).toBe(false); }); + it("matchPattern: accepts bounded nested repetition", async () => { + const { matchPattern } = await import("../packages/vinext/src/server/middleware.js"); + // Mirrors Next.js/path-to-regexp: finite repetition inside a finite group + // is not an unbounded backtracking risk. + const matcher = "/archive/:date(\\d{4}(?:-\\d{2}){2})"; + expect(matchPattern("/archive/2024-07-10", matcher)).toBe(true); + expect(matchPattern("/archive/2024-7-10", matcher)).toBe(false); + expect(matchPattern("/archive/2024-07-10-11", matcher)).toBe(false); + }); + + it("rejects prefix-ambiguous alternatives only under unbounded repetition", async () => { + const { compileMiddlewareMatcherPattern } = + await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); + + for (const matcher of ["/:path((?:a|aa)+)", "/:path((?:aa|a)*)", "/:path((?:[a]|aa){1,})"]) { + expect(compileMiddlewareMatcherPattern(matcher), matcher).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("ambiguous alternatives"), + }); + } + + expect(compileMiddlewareMatcherPattern("/:path((?:a|aa){2})").regexp).toBeInstanceOf(RegExp); + expect(compileMiddlewareMatcherPattern("/:path((?:foo|bar)+)").regexp).toBeInstanceOf(RegExp); + }); + // Ported from Next.js path-to-regexp matcher semantics: // packages/next/src/lib/try-to-parse-path.ts // https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/try-to-parse-path.ts @@ -10793,6 +10819,13 @@ describe("isSafeRegex", () => { expect(isSafeRegex("(\\d{2,4})")).toBe(true); }); + it("accepts bounded groups containing bounded repetition", async () => { + const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); + expect(isSafeRegex("\\d{4}(?:-\\d{2}){2}")).toBe(true); + expect(isSafeRegex("(?:a{1,2}){3}")).toBe(true); + expect(isSafeRegex("(?:a{1,2}){2,4}")).toBe(true); + }); + it("rejects nested quantifiers: (a+)+", async () => { const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); expect(isSafeRegex("(a+)+")).toBe(false); @@ -10823,6 +10856,12 @@ describe("isSafeRegex", () => { expect(isSafeRegex("(a+){2,}")).toBe(false); }); + it("accepts bounded repetition around an unbounded inner atom", async () => { + const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); + expect(isSafeRegex("(a+){2}")).toBe(true); + expect(isSafeRegex("(a+){2,4}")).toBe(true); + }); + it("accepts quantifier on group without inner quantifier", async () => { const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); // (ab)+ is fine — no inner quantifier @@ -11414,6 +11453,58 @@ describe("checkHasConditions", () => { ).toBe(false); }); + // Ported from Next.js matchHas, which uses `value.slice(-1)[0]` when the + // parsed query contains duplicate keys. + // https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/prepare-destination.ts + it("uses the last duplicate query value for has and missing", async () => { + const { checkHasConditions } = await import("../packages/vinext/src/config/config-matchers.js"); + const ctx = { + ...makeCtx(), + query: new URLSearchParams("role=guest&role=admin&blocked=1&blocked=0"), + }; + + expect( + checkHasConditions([{ type: "query", key: "role", value: "admin" }], undefined, ctx), + ).toBe(true); + expect( + checkHasConditions([{ type: "query", key: "role", value: "guest" }], undefined, ctx), + ).toBe(false); + expect( + checkHasConditions(undefined, [{ type: "query", key: "blocked", value: "1" }], ctx), + ).toBe(true); + expect( + checkHasConditions(undefined, [{ type: "query", key: "blocked", value: "0" }], ctx), + ).toBe(false); + }); + + it('treats value: "" as presence-only for every condition type', async () => { + const { checkHasConditions } = await import("../packages/vinext/src/config/config-matchers.js"); + const ctx = makeCtx({ + headers: { "x-present": "yes" }, + cookies: { session: "active" }, + query: { preview: "enabled" }, + host: "example.com", + }); + const present = [ + { type: "header" as const, key: "x-present", value: "" }, + { type: "cookie" as const, key: "session", value: "" }, + { type: "query" as const, key: "preview", value: "" }, + { type: "host" as const, key: "", value: "" }, + ]; + + expect(checkHasConditions(present, undefined, ctx)).toBe(true); + for (const condition of present) { + expect(checkHasConditions(undefined, [condition], ctx), condition.type).toBe(false); + } + + expect( + checkHasConditions([{ type: "query", key: "empty", value: "" }], undefined, { + ...ctx, + query: new URLSearchParams("empty="), + }), + ).toBe(false); + }); + // -- host conditions -- it("has host: matches exact value", async () => { const { checkHasConditions } = await import("../packages/vinext/src/config/config-matchers.js"); From 226685f9701b1db5f9ad5ce9ad61055801c30616 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:20:34 +0100 Subject: [PATCH 4/9] fix(middleware): make matcher analysis deterministic --- packages/vinext/src/config/config-matchers.ts | 138 +---- .../src/server/middleware-matcher-pattern.ts | 201 +------ packages/vinext/src/utils/regex-safety.ts | 558 ++++++++++++++++++ .../app/codes/[value]/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 1 + .../middleware-matcher-redos-child.ts | 21 + tests/middleware-matcher-auth.test.ts | 11 +- tests/shims.test.ts | 43 +- 8 files changed, 647 insertions(+), 329 deletions(-) create mode 100644 packages/vinext/src/utils/regex-safety.ts create mode 100644 tests/fixtures/middleware-matcher-auth/app/codes/[value]/page.tsx diff --git a/packages/vinext/src/config/config-matchers.ts b/packages/vinext/src/config/config-matchers.ts index 30ee496471..6a07bac016 100644 --- a/packages/vinext/src/config/config-matchers.ts +++ b/packages/vinext/src/config/config-matchers.ts @@ -21,6 +21,7 @@ import { } from "../utils/protocol-headers.js"; import { buildRequestHeadersFromMiddlewareResponse } from "../utils/middleware-request-headers.js"; import { parseCookieHeader } from "../utils/parse-cookie.js"; +import { analyzeRegexSafety } from "../utils/regex-safety.js"; /** * Cache for compiled regex patterns in matchConfigPattern. @@ -265,129 +266,16 @@ function stripHopByHopRequestHeaders(headers: Headers): void { /** * Detect regex patterns vulnerable to catastrophic backtracking (ReDoS). * - * Uses a lightweight heuristic: scans the pattern string for nested quantifiers - * (a quantifier applied to a group that itself contains a quantifier). This - * catches the most common pathological patterns like `(a+)+`, `(.*)*`, - * `([^/]+)+`, `(a|a+)+` without needing a full regex parser. + * Uses the same deterministic structural analysis as middleware matcher + * validation. Nested bounded repetition is accepted only when its repeated + * language has fixed width and unambiguous branches; a fixed outer count can + * otherwise still cause polynomially catastrophic backtracking on long near + * misses. * * Returns true if the pattern appears safe, false if it's potentially dangerous. */ -export function isSafeRegex(pattern: string): boolean { - // Track parenthesis nesting depth and whether we've seen a quantifier - // at each depth level. - const quantifierAtDepth: boolean[] = []; - let depth = 0; - let i = 0; - - while (i < pattern.length) { - const ch = pattern[i]; - - // Skip escaped characters - if (ch === "\\") { - i += 2; - continue; - } - - // Skip character classes [...] — quantifiers inside them are literal - if (ch === "[") { - i++; - while (i < pattern.length && pattern[i] !== "]") { - if (pattern[i] === "\\") i++; // skip escaped char in class - i++; - } - i++; // skip closing ] - continue; - } - - if (ch === "(") { - depth++; - // Initialize: no quantifier seen yet at this new depth - if (quantifierAtDepth.length <= depth) { - quantifierAtDepth.push(false); - } else { - quantifierAtDepth[depth] = false; - } - i++; - continue; - } - - if (ch === ")") { - const hadQuantifier = depth > 0 && quantifierAtDepth[depth]; - if (depth > 0) depth--; - - // Only unbounded repetition of a group with inner quantifiers is - // intrinsically dangerous. A finite group repetition such as - // `(?:-\d{2}){2}` has a bounded number of paths and is safe by itself. - // Still propagate every group quantifier to the enclosing group so a - // later unbounded repetition around it is rejected. - const next = pattern[i + 1]; - let hasGroupQuantifier = next === "+" || next === "*" || next === "?"; - let hasUnboundedGroupQuantifier = next === "+" || next === "*"; - if (next === "{") { - const braceQuantifier = /^\{\d+(,\d*)?\}/.exec(pattern.slice(i + 1)); - hasGroupQuantifier = braceQuantifier !== null; - hasUnboundedGroupQuantifier = braceQuantifier?.[1]?.endsWith(",") === true; - } - if (hasGroupQuantifier) { - if (hasUnboundedGroupQuantifier && hadQuantifier) { - // Nested quantifier detected: quantifier on a group that contains a quantifier - return false; - } - // Mark the enclosing depth as having a quantifier - if (depth >= 0 && depth < quantifierAtDepth.length) { - quantifierAtDepth[depth] = true; - } - } - i++; - continue; - } - - // Detect quantifiers: +, *, ?, {n,m} - // '?' is a quantifier (optional) unless it follows another quantifier (+, *, ?, }) - // in which case it's a non-greedy modifier. - if (ch === "+" || ch === "*") { - if (depth > 0) { - quantifierAtDepth[depth] = true; - } - i++; - continue; - } - - if (ch === "?") { - // Group prefixes such as (?:...), (?=...), (?!...), and (?<=...) are - // syntax markers, not optional quantifiers. - if (i > 0 && pattern[i - 1] === "(" && /[:=!<]/.test(pattern[i + 1] ?? "")) { - i++; - continue; - } - // '?' after +, *, ?, or } is a non-greedy modifier, not a quantifier - const prev = i > 0 ? pattern[i - 1] : ""; - if (prev !== "+" && prev !== "*" && prev !== "?" && prev !== "}") { - if (depth > 0) { - quantifierAtDepth[depth] = true; - } - } - i++; - continue; - } - - if (ch === "{") { - // Check if this is a quantifier {n}, {n,}, {n,m} - let j = i + 1; - while (j < pattern.length && /[\d,]/.test(pattern[j])) j++; - if (j < pattern.length && pattern[j] === "}" && j > i + 1) { - if (depth > 0) { - quantifierAtDepth[depth] = true; - } - i = j + 1; - continue; - } - } - - i++; - } - - return true; +export function isSafeRegex(pattern: string, flags?: string): boolean { + return analyzeRegexSafety(pattern, { ignoreCase: flags?.includes("i") }) === null; } /** @@ -397,11 +285,11 @@ export function isSafeRegex(pattern: string): boolean { * Logs a warning when a pattern is rejected so developers can fix their config. */ export function safeRegExp(pattern: string, flags?: string): RegExp | null { - if (!isSafeRegex(pattern)) { + if (!isSafeRegex(pattern, flags)) { console.warn( `[vinext] Rejecting potentially unsafe regex pattern (ReDoS risk): ${pattern}\n` + - ` Patterns with nested quantifiers (e.g. (a+)+) can cause catastrophic backtracking.\n` + - ` Simplify the pattern to avoid nested repetition.`, + ` Nested or ambiguous repetition can cause catastrophic backtracking.\n` + + ` Simplify the pattern to make repeated matches fixed and unambiguous.`, ); return null; } @@ -676,6 +564,10 @@ function matchSingleCondition( case "query": { const queryValues = ctx.query.getAll(condition.key); if (queryValues.length === 0) return null; + // Next.js checks presence against the parsed value before selecting the + // last array element for a value regex. A duplicate key is represented + // as a truthy array even when its final value is empty. + if (!condition.value && queryValues.length > 1) return _emptyParams(); // Node parses duplicate query keys as an array and Next.js matchHas // explicitly tests its final value (`value.slice(-1)[0]`). return _matchConditionValue(queryValues[queryValues.length - 1], condition.value); diff --git a/packages/vinext/src/server/middleware-matcher-pattern.ts b/packages/vinext/src/server/middleware-matcher-pattern.ts index 66e8cf6b6a..a74f40325d 100644 --- a/packages/vinext/src/server/middleware-matcher-pattern.ts +++ b/packages/vinext/src/server/middleware-matcher-pattern.ts @@ -1,5 +1,5 @@ -import { isSafeRegex } from "../config/config-matchers.js"; import type { HasCondition } from "../config/next-config.js"; +import { analyzeRegexSafety, regexAtomsMayOverlap } from "../utils/regex-safety.js"; import { middlewarePathTokensToRegExp, normalizeMiddlewarePathTokens, @@ -105,193 +105,7 @@ function patternMatches(pattern: string, value: string): boolean { } function atomsOverlap(left: string, right: string): boolean { - let leftRegexp: RegExp; - let rightRegexp: RegExp; - try { - leftRegexp = new RegExp(`^(?:${left})$`); - rightRegexp = new RegExp(`^(?:${right})$`); - } catch { - return true; - } - - // Middleware paths are URL text. The ASCII URL alphabet is sufficient to - // prove overlap for the common literal, dot, shorthand, and class atoms; - // include one non-ASCII code point for broad Unicode classes. - for (let code = 0x20; code <= 0x7e; code++) { - const character = String.fromCharCode(code); - if (leftRegexp.test(character) && rightRegexp.test(character)) return true; - } - return leftRegexp.test("é") && rightRegexp.test("é"); -} - -function findGroupEnd(pattern: string, start: number): number | null { - let depth = 0; - let inClass = false; - for (let index = start; index < pattern.length; index++) { - const character = pattern[index]; - if (character === "\\") { - index++; - continue; - } - if (character === "[") { - inClass = true; - continue; - } - if (character === "]" && inClass) { - inClass = false; - continue; - } - if (inClass) continue; - if (character === "(") depth++; - if (character === ")" && --depth === 0) return index; - } - return null; -} - -function splitTopLevelAlternatives(pattern: string): string[] { - const alternatives: string[] = []; - let start = 0; - let depth = 0; - let inClass = false; - for (let index = 0; index < pattern.length; index++) { - const character = pattern[index]; - if (character === "\\") { - index++; - continue; - } - if (character === "[") { - inClass = true; - continue; - } - if (character === "]" && inClass) { - inClass = false; - continue; - } - if (inClass) continue; - if (character === "(") depth++; - else if (character === ")") depth--; - else if (character === "|" && depth === 0) { - alternatives.push(pattern.slice(start, index)); - start = index + 1; - } - } - alternatives.push(pattern.slice(start)); - return alternatives; -} - -function firstConsumingAtoms(branch: string): string[] | null { - for (let index = 0; index < branch.length; index++) { - const character = branch[index]; - if (character === "^" || character === "$") continue; - if (character === "\\") { - return index + 1 < branch.length ? [branch.slice(index, index + 2)] : null; - } - if (character === "[") { - let end = index + 1; - if (branch[end] === "^") end++; - while (end < branch.length && branch[end] !== "]") { - if (branch[end] === "\\") end++; - end++; - } - return end < branch.length ? [branch.slice(index, end + 1)] : null; - } - if (character === ".") return ["."]; - if (character === "(") { - const end = findGroupEnd(branch, index); - if (end === null) return null; - const body = branch.slice(index + 1, end); - if (/^\?(?:[=!]|<[=!])/.test(body)) { - // Lookarounds consume no text, so the following atom owns the prefix. - index = end; - continue; - } - const groupBody = body.startsWith("?:") ? body.slice(2) : body; - const alternatives = splitTopLevelAlternatives(groupBody); - const atoms = alternatives.flatMap((alternative) => firstConsumingAtoms(alternative) ?? []); - return atoms.length === alternatives.length ? atoms : null; - } - if ("*+?{}|)".includes(character)) return null; - return [character]; - } - return null; -} - -function alternativesMaySharePrefix(pattern: string): boolean { - const alternatives = splitTopLevelAlternatives(pattern); - if (alternatives.length > 1) { - const firstAtoms = alternatives.map(firstConsumingAtoms); - for (let left = 0; left < firstAtoms.length; left++) { - for (let right = left + 1; right < firstAtoms.length; right++) { - const leftAtoms = firstAtoms[left]; - const rightAtoms = firstAtoms[right]; - // Unknown or empty branches cannot be proven prefix-disjoint, so fail - // closed when the alternation itself is repeated without a bound. - if (!leftAtoms || !rightAtoms) return true; - if (leftAtoms.some((a) => rightAtoms.some((b) => atomsOverlap(a, b)))) return true; - } - } - } - - // An alternation nested inside an unbounded repeated group can still make - // the repeated language ambiguous even when wrapped in a non-capturing - // group or preceded by shared literals. Inspect all child groups as well. - for (let index = 0; index < pattern.length; index++) { - if (pattern[index] === "\\") { - index++; - continue; - } - if (pattern[index] === "[") { - while (++index < pattern.length && pattern[index] !== "]") { - if (pattern[index] === "\\") index++; - } - continue; - } - if (pattern[index] !== "(") continue; - const end = findGroupEnd(pattern, index); - if (end === null) return true; - const body = pattern.slice(index + 1, end); - if (!/^\?(?:[=!]|<[=!])/.test(body)) { - const groupBody = body.startsWith("?:") ? body.slice(2) : body; - if (alternativesMaySharePrefix(groupBody)) return true; - } - index = end; - } - return false; -} - -function hasAmbiguousQuantifiedAlternation(pattern: string): boolean { - const groupStarts: number[] = []; - let inClass = false; - for (let index = 0; index < pattern.length; index++) { - const character = pattern[index]; - if (character === "\\") { - index++; - continue; - } - if (character === "[") { - inClass = true; - continue; - } - if (character === "]" && inClass) { - inClass = false; - continue; - } - if (inClass) continue; - if (character === "(") { - groupStarts.push(index); - continue; - } - if (character !== ")") continue; - const start = groupStarts.pop(); - if (start === undefined) return true; - const suffix = pattern.slice(index + 1); - const unbounded = suffix[0] === "+" || suffix[0] === "*" || /^\{\d+,\}/.test(suffix); - if (!unbounded) continue; - const body = pattern.slice(start + 1, index); - const groupBody = body.startsWith("?:") ? body.slice(2) : body; - if (alternativesMaySharePrefix(groupBody)) return true; - } - return groupStarts.length > 0; + return regexAtomsMayOverlap(left, right, true); } function groupCanMatchEmpty(group: string): boolean { @@ -397,11 +211,12 @@ function hasOverlappingSequentialRepetition(pattern: string): boolean { } function unsafeTokenReason(token: MiddlewarePathKey): string | null { - if (!isSafeRegex(token.pattern)) { - return `parameter "${token.name}" contains nested repetition`; - } - if (hasAmbiguousQuantifiedAlternation(token.pattern)) { - return `parameter "${token.name}" contains ambiguous alternatives under unbounded repetition`; + const regexSafetyIssue = analyzeRegexSafety(token.pattern, { ignoreCase: true }); + if (regexSafetyIssue) { + if (regexSafetyIssue === "analysis budget exceeded") { + return `parameter "${token.name}" exceeds the regex analysis budget`; + } + return `parameter "${token.name}" contains ${regexSafetyIssue}`; } if (hasOverlappingSequentialRepetition(token.pattern)) { return `parameter "${token.name}" contains overlapping sequential repetition`; diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts new file mode 100644 index 0000000000..426bd58d52 --- /dev/null +++ b/packages/vinext/src/utils/regex-safety.ts @@ -0,0 +1,558 @@ +/** + * Deterministic structural analysis for request-facing regular expressions. + * + * The parser derives exact widths and finite branch words without executing + * attacker-sized probes. Repeated finite languages are checked with a prefix + * trie, so literal alternatives are linear in their total source length. + * Unsupported intersections fail closed behind explicit node, word, symbol, + * comparison, and nesting budgets. + */ +type RegexNode = + | { kind: "atom"; symbol: RegexSymbol | null; fixedWidth: boolean } + | { kind: "assertion"; child: RegexNode } + | { kind: "sequence"; children: RegexNode[] } + | { kind: "alternation"; branches: RegexNode[] } + | { kind: "repeat"; child: RegexNode; min: number; max: number }; + +type RegexSymbol = + | { kind: "literal"; key: string; value: string } + | { kind: "opaque"; key: string; pattern: string; ignoreCase: boolean }; + +export type RegexSafetyIssue = + | "nested repetition" + | "ambiguous alternatives under repetition" + | "analysis budget exceeded"; + +const MAX_NODES = 16_384; +const MAX_NESTING_DEPTH = 256; +const MAX_PATTERN_LENGTH = 65_536; +const MAX_WORDS = 4_096; +const MAX_WORD_SYMBOLS = 32_768; +const MAX_OPAQUE_COMPARISONS = 4_096; + +function canonicalizeIgnoreCase(character: string): string { + const upper = character.toUpperCase(); + // ECMAScript's non-Unicode Canonicalize operation keeps the original UTF-16 + // code unit when uppercasing expands it or maps a non-ASCII character to + // ASCII. Middleware regexes are compiled with `i`, but not `u`. + if (upper.length !== 1) return character; + if (character.charCodeAt(0) >= 0x80 && upper.charCodeAt(0) < 0x80) return character; + return upper; +} + +function literalSymbol(character: string, ignoreCase: boolean): RegexSymbol { + const key = ignoreCase ? canonicalizeIgnoreCase(character) : character; + return { kind: "literal", key, value: character }; +} + +class RegexParser { + index = 0; + nodes = 0; + depth = 0; + exceededBudget = false; + + constructor( + private readonly pattern: string, + private readonly ignoreCase: boolean, + ) {} + + parse(): RegexNode { + return this.parseAlternation(); + } + + private node(node: T): T { + this.nodes++; + if (this.nodes > MAX_NODES) this.exceededBudget = true; + return node; + } + + private parseAlternation(): RegexNode { + const branches = [this.parseSequence()]; + while (this.pattern[this.index] === "|") { + this.index++; + branches.push(this.parseSequence()); + } + return branches.length === 1 ? branches[0] : this.node({ kind: "alternation", branches }); + } + + private parseSequence(): RegexNode { + const children: RegexNode[] = []; + while (this.index < this.pattern.length) { + const character = this.pattern[this.index]; + if (character === "|" || character === ")") break; + children.push(this.parseTerm()); + } + return children.length === 1 ? children[0] : this.node({ kind: "sequence", children }); + } + + private parseTerm(): RegexNode { + const atom = this.parseAtom(); + const quantifier = this.parseQuantifier(); + if (!quantifier) return atom; + if (this.pattern[this.index] === "?") this.index++; + return this.node({ kind: "repeat", child: atom, ...quantifier }); + } + + private parseAtom(): RegexNode { + const character = this.pattern[this.index++]; + if (character === "(") return this.parseGroup(); + if (character === "[") return this.parseClass(); + if (character === "\\") return this.parseEscape(); + if (character === "^" || character === "$") { + return this.node({ kind: "assertion", child: this.node({ kind: "sequence", children: [] }) }); + } + if (character === ".") { + return this.node({ + kind: "atom", + symbol: { kind: "opaque", key: ".", pattern: ".", ignoreCase: this.ignoreCase }, + fixedWidth: true, + }); + } + return this.node({ + kind: "atom", + symbol: literalSymbol(character, this.ignoreCase), + fixedWidth: true, + }); + } + + private parseGroup(): RegexNode { + this.depth++; + if (this.depth > MAX_NESTING_DEPTH) { + this.exceededBudget = true; + this.skipGroup(); + this.depth--; + return this.node({ kind: "atom", symbol: null, fixedWidth: false }); + } + let assertion = false; + if (this.pattern[this.index] === "?") { + const marker = this.pattern[this.index + 1]; + if (marker === ":") { + this.index += 2; + } else if (marker === "=" || marker === "!") { + assertion = true; + this.index += 2; + } else if ( + marker === "<" && + (this.pattern[this.index + 2] === "=" || this.pattern[this.index + 2] === "!") + ) { + assertion = true; + this.index += 3; + } else if (marker === "<") { + const nameEnd = this.pattern.indexOf(">", this.index + 2); + this.index = nameEnd === -1 ? this.pattern.length : nameEnd + 1; + } else { + // Unsupported group prefixes will be rejected by RegExp compilation. + // Keep analysis conservative if this parser is asked to inspect one. + while (this.index < this.pattern.length && this.pattern[this.index] !== ")") this.index++; + if (this.pattern[this.index] === ")") this.index++; + this.depth--; + return this.node({ kind: "atom", symbol: null, fixedWidth: false }); + } + } + + const child = this.parseAlternation(); + if (this.pattern[this.index] === ")") this.index++; + this.depth--; + return assertion ? this.node({ kind: "assertion", child }) : child; + } + + private skipGroup(): void { + let depth = 1; + let inClass = false; + while (this.index < this.pattern.length && depth > 0) { + const character = this.pattern[this.index++]; + if (character === "\\") { + this.index++; + continue; + } + if (character === "[") inClass = true; + else if (character === "]") inClass = false; + else if (!inClass && character === "(") depth++; + else if (!inClass && character === ")") depth--; + } + } + + private parseClass(): RegexNode { + const start = this.index - 1; + while (this.index < this.pattern.length) { + const character = this.pattern[this.index++]; + if (character === "\\") this.index++; + else if (character === "]") break; + } + const raw = this.pattern.slice(start, this.index); + return this.node({ + kind: "atom", + symbol: { kind: "opaque", key: raw, pattern: raw, ignoreCase: this.ignoreCase }, + fixedWidth: true, + }); + } + + private parseEscape(): RegexNode { + const escaped = this.pattern[this.index++]; + if (escaped === undefined) { + return this.node({ kind: "atom", symbol: null, fixedWidth: false }); + } + if (escaped === "b" || escaped === "B") { + return this.node({ kind: "assertion", child: this.node({ kind: "sequence", children: [] }) }); + } + if (/\d/.test(escaped)) { + return this.node({ kind: "atom", symbol: null, fixedWidth: false }); + } + + let literal: string | null = null; + if (escaped === "x" && /^[\da-fA-F]{2}/.test(this.pattern.slice(this.index, this.index + 2))) { + literal = String.fromCharCode( + Number.parseInt(this.pattern.slice(this.index, this.index + 2), 16), + ); + this.index += 2; + } else if ( + escaped === "u" && + /^[\da-fA-F]{4}/.test(this.pattern.slice(this.index, this.index + 4)) + ) { + literal = String.fromCharCode( + Number.parseInt(this.pattern.slice(this.index, this.index + 4), 16), + ); + this.index += 4; + } else if ("nrtvf0".includes(escaped)) { + literal = ({ n: "\n", r: "\r", t: "\t", v: "\v", f: "\f", 0: "\0" } as const)[ + escaped as "n" | "r" | "t" | "v" | "f" | "0" + ]; + } else if (!/[A-Za-z]/.test(escaped)) { + literal = escaped; + } + + if (literal !== null) { + return this.node({ + kind: "atom", + symbol: literalSymbol(literal, this.ignoreCase), + fixedWidth: true, + }); + } + const raw = `\\${escaped}`; + return this.node({ + kind: "atom", + symbol: { kind: "opaque", key: raw, pattern: raw, ignoreCase: this.ignoreCase }, + fixedWidth: true, + }); + } + + private parseQuantifier(): { min: number; max: number } | null { + const character = this.pattern[this.index]; + if (character === "*") { + this.index++; + return { min: 0, max: Infinity }; + } + if (character === "+") { + this.index++; + return { min: 1, max: Infinity }; + } + if (character === "?") { + this.index++; + return { min: 0, max: 1 }; + } + if (character !== "{") return null; + + const start = this.index; + let cursor = start + 1; + while (/\d/.test(this.pattern[cursor] ?? "")) cursor++; + if (cursor === start + 1) return null; + const min = Number(this.pattern.slice(start + 1, cursor)); + if (this.pattern[cursor] === "}") { + this.index = cursor + 1; + return { min, max: min }; + } + if (this.pattern[cursor] !== ",") return null; + cursor++; + const maxStart = cursor; + while (/\d/.test(this.pattern[cursor] ?? "")) cursor++; + if (this.pattern[cursor] !== "}") return null; + const max = cursor === maxStart ? Infinity : Number(this.pattern.slice(maxStart, cursor)); + this.index = cursor + 1; + return { min, max }; + } +} + +function exactWidth(node: RegexNode): number | null { + switch (node.kind) { + case "atom": + return node.fixedWidth ? 1 : null; + case "assertion": + return 0; + case "sequence": { + let width = 0; + for (const child of node.children) { + const childWidth = exactWidth(child); + if (childWidth === null) return null; + width += childWidth; + } + return width; + } + case "alternation": { + let width: number | null | undefined; + for (const branch of node.branches) { + const branchWidth = exactWidth(branch); + if (branchWidth === null) return null; + if (width === undefined) width = branchWidth; + else if (width !== branchWidth) return null; + } + return width ?? 0; + } + case "repeat": { + if (node.min !== node.max || !Number.isFinite(node.max)) return null; + const childWidth = exactWidth(node.child); + return childWidth === null ? null : childWidth * node.min; + } + } +} + +function containsConsumingRepetition(node: RegexNode): boolean { + switch (node.kind) { + case "atom": + return false; + case "assertion": + return false; + case "sequence": + return node.children.some(containsConsumingRepetition); + case "alternation": + return node.branches.some(containsConsumingRepetition); + case "repeat": + return node.min !== 1 || node.max !== 1 || containsConsumingRepetition(node.child); + } +} + +function containsConsumingAlternation(node: RegexNode): boolean { + switch (node.kind) { + case "atom": + return false; + case "assertion": + return false; + case "sequence": + return node.children.some(containsConsumingAlternation); + case "alternation": + return true; + case "repeat": + return containsConsumingAlternation(node.child); + } +} + +type WordBudget = { words: number; symbols: number; exceeded: boolean }; + +function fixedWords(node: RegexNode, budget: WordBudget): RegexSymbol[][] | null { + if (budget.exceeded) return null; + switch (node.kind) { + case "atom": + return node.symbol ? [[node.symbol]] : null; + case "assertion": + return [[]]; + case "sequence": { + let words: RegexSymbol[][] = [[]]; + for (const child of node.children) { + const childWords = fixedWords(child, budget); + if (!childWords) return null; + const next: RegexSymbol[][] = []; + for (const prefix of words) { + for (const suffix of childWords) { + if (++budget.words > MAX_WORDS) { + budget.exceeded = true; + return null; + } + const word = [...prefix, ...suffix]; + budget.symbols += word.length; + if (budget.symbols > MAX_WORD_SYMBOLS) { + budget.exceeded = true; + return null; + } + next.push(word); + } + } + words = next; + } + return words; + } + case "alternation": { + const words: RegexSymbol[][] = []; + for (const branch of node.branches) { + const branchWords = fixedWords(branch, budget); + if (!branchWords) return null; + words.push(...branchWords); + if ((budget.words += branchWords.length) > MAX_WORDS) { + budget.exceeded = true; + return null; + } + } + return words; + } + case "repeat": { + if (node.min !== node.max || !Number.isFinite(node.max)) return null; + let words: RegexSymbol[][] = [[]]; + const childWords = fixedWords(node.child, budget); + if (!childWords) return null; + for (let count = 0; count < node.min; count++) { + const next: RegexSymbol[][] = []; + for (const prefix of words) { + for (const suffix of childWords) { + if (++budget.words > MAX_WORDS) { + budget.exceeded = true; + return null; + } + const word = [...prefix, ...suffix]; + budget.symbols += word.length; + if (budget.symbols > MAX_WORD_SYMBOLS) { + budget.exceeded = true; + return null; + } + next.push(word); + } + } + words = next; + } + return words; + } + } +} + +type TrieEdge = { symbol: RegexSymbol; node: TrieNode }; +type TrieNode = { + terminal: boolean; + literals: Map; + opaque: TrieEdge[]; +}; + +function createTrieNode(): TrieNode { + return { terminal: false, literals: new Map(), opaque: [] }; +} + +function opaqueMatchesLiteral(opaque: RegexSymbol, literal: RegexSymbol): boolean { + if (opaque.kind !== "opaque" || literal.kind !== "literal") return false; + try { + return new RegExp(`^(?:${opaque.pattern})$`, opaque.ignoreCase ? "i" : "").test(literal.value); + } catch { + return true; + } +} + +function insertPrefixFreeWord( + root: TrieNode, + word: RegexSymbol[], + comparisons: { count: number }, +): boolean { + let node = root; + for (const symbol of word) { + if (node.terminal) return false; + let edge: TrieEdge | undefined; + if (symbol.kind === "literal") { + edge = node.literals.get(symbol.key); + for (const opaque of node.opaque) { + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; + if (opaqueMatchesLiteral(opaque.symbol, symbol)) return false; + } + if (!edge) { + edge = { symbol, node: createTrieNode() }; + node.literals.set(symbol.key, edge); + } + } else { + for (const literal of node.literals.values()) { + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; + if (opaqueMatchesLiteral(symbol, literal.symbol)) return false; + } + edge = node.opaque.find((candidate) => candidate.symbol.key === symbol.key); + if (!edge && node.opaque.length > 0) return false; + if (!edge) { + edge = { symbol, node: createTrieNode() }; + node.opaque.push(edge); + } + } + node = edge.node; + } + if (node.terminal || node.literals.size > 0 || node.opaque.length > 0) return false; + node.terminal = true; + return true; +} + +function hasPrefixFreeFiniteLanguage(node: RegexNode): { safe: boolean; budgetExceeded: boolean } { + const budget: WordBudget = { words: 0, symbols: 0, exceeded: false }; + const words = fixedWords(node, budget); + if (!words) return { safe: false, budgetExceeded: budget.exceeded }; + const root = createTrieNode(); + const comparisons = { count: 0 }; + for (const word of words) { + if (!insertPrefixFreeWord(root, word, comparisons)) { + return { + safe: false, + budgetExceeded: comparisons.count > MAX_OPAQUE_COMPARISONS, + }; + } + } + return { safe: true, budgetExceeded: false }; +} + +function findSafetyIssue(node: RegexNode): RegexSafetyIssue | null { + switch (node.kind) { + case "atom": + return null; + case "assertion": + return findSafetyIssue(node.child); + case "sequence": + for (const child of node.children) { + const issue = findSafetyIssue(child); + if (issue) return issue; + } + return null; + case "alternation": + for (const branch of node.branches) { + const issue = findSafetyIssue(branch); + if (issue) return issue; + } + return null; + case "repeat": { + const nestedRepetition = containsConsumingRepetition(node.child); + if (node.max === Infinity && nestedRepetition) return "nested repetition"; + if (node.max > 1 && nestedRepetition && exactWidth(node.child) === null) { + return "nested repetition"; + } + if (node.max > 1 && containsConsumingAlternation(node.child)) { + const prefixFree = hasPrefixFreeFiniteLanguage(node.child); + if (!prefixFree.safe) { + return prefixFree.budgetExceeded + ? "analysis budget exceeded" + : "ambiguous alternatives under repetition"; + } + } + return findSafetyIssue(node.child); + } + } +} + +export function analyzeRegexSafety( + pattern: string, + options: { ignoreCase?: boolean } = {}, +): RegexSafetyIssue | null { + if (pattern.length > MAX_PATTERN_LENGTH) return "analysis budget exceeded"; + const parser = new RegexParser(pattern, options.ignoreCase === true); + const node = parser.parse(); + if (parser.exceededBudget) return "analysis budget exceeded"; + return findSafetyIssue(node); +} + +export function regexAtomsMayOverlap(left: string, right: string, ignoreCase = false): boolean { + const leftParser = new RegexParser(left, ignoreCase); + const rightParser = new RegexParser(right, ignoreCase); + const leftNode = leftParser.parse(); + const rightNode = rightParser.parse(); + const leftWords = fixedWords(leftNode, { words: 0, symbols: 0, exceeded: false }); + const rightWords = fixedWords(rightNode, { words: 0, symbols: 0, exceeded: false }); + if (!leftWords || !rightWords || leftWords.length !== 1 || rightWords.length !== 1) return true; + const leftSymbol = leftWords[0][0]; + const rightSymbol = rightWords[0][0]; + if (!leftSymbol || !rightSymbol) return true; + if (leftSymbol.kind === "literal" && rightSymbol.kind === "literal") { + return leftSymbol.key === rightSymbol.key; + } + if (leftSymbol.kind === "opaque" && rightSymbol.kind === "literal") { + return opaqueMatchesLiteral(leftSymbol, rightSymbol); + } + if (leftSymbol.kind === "literal" && rightSymbol.kind === "opaque") { + return opaqueMatchesLiteral(rightSymbol, leftSymbol); + } + return true; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/codes/[value]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/codes/[value]/page.tsx new file mode 100644 index 0000000000..5dc59c3f99 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/codes/[value]/page.tsx @@ -0,0 +1,3 @@ +export default function CodesPage() { + return
codes secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts index 8166b7a2eb..850a48c886 100644 --- a/tests/fixtures/middleware-matcher-auth/middleware.ts +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -17,6 +17,7 @@ export const config = { "/(foo.*|bar)/:path*", "/report{.:ext}", "/archive/:date(\\d{4}(?:-\\d{2}){2})", + "/codes/:value((?:ab|ac)+)", { source: "/conditioned", has: [ diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index b3c108b1b2..2b37a623f9 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -1,4 +1,6 @@ +import { performance } from "node:perf_hooks"; import { matchPattern } from "../../packages/vinext/src/server/middleware-matcher.ts"; +import { analyzeRegexSafety } from "../../packages/vinext/src/utils/regex-safety.ts"; const nearMiss = `/${"a/".repeat(2_000)}not-end`; for (const modifier of ["*", "+"]) { @@ -22,3 +24,22 @@ const ambiguousAlternative = "/:path((?:a|aa)+)"; if (!matchPattern(`/${"a".repeat(3_000)}b`, ambiguousAlternative)) { throw new Error(`Unsafe matcher did not fail closed: ${ambiguousAlternative}`); } + +for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { + if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { + throw new Error(`Unsafe matcher did not fail closed: ${matcher}`); + } +} + +// Keep the analysis itself linear for large, disjoint literal alternations. +// CJK literals have stable, distinct non-Unicode ignore-case canonical forms. +const alternatives = Array.from({ length: 2_000 }, (_, index) => + String.fromCharCode(0x4e00 + index), +).join("|"); +const analysisStart = performance.now(); +const analysisIssue = analyzeRegexSafety(`(?:${alternatives})+`, { ignoreCase: true }); +const analysisDuration = performance.now() - analysisStart; +if (analysisIssue) throw new Error(`Safe large alternation was rejected: ${analysisIssue}`); +if (analysisDuration > 1_000) { + throw new Error(`Large alternation analysis took ${analysisDuration.toFixed(1)}ms`); +} diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 1d6286ffdd..98d1659ddb 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -35,6 +35,7 @@ const PROTECTED_PATHS = [ "/bar/secret", "/report.json", "/archive/2024-07-10", + "/codes/abac", ] as const; async function assertAuthGuard(baseUrl: string): Promise { @@ -74,14 +75,14 @@ async function assertAuthGuard(baseUrl: string): Promise { cookie: "session=active", }; const conditioned = await fetch( - `${baseUrl}/conditioned?role=guest&role=admin&present=yes&blocked=1&blocked=0`, + `${baseUrl}/conditioned?role=guest&role=admin&present=yes&present=&blocked=1&blocked=0`, { headers: conditionedHeaders }, ); expect(conditioned.status, await conditioned.text()).toBe(403); expect(conditioned.headers.get("x-auth-guard")).toBe("blocked"); const wrongLastHas = await fetch( - `${baseUrl}/conditioned?role=admin&role=guest&present=yes&blocked=1&blocked=0`, + `${baseUrl}/conditioned?role=admin&role=guest&present=yes&present=&blocked=1&blocked=0`, { headers: conditionedHeaders }, ); const wrongLastHasBody = await wrongLastHas.text(); @@ -90,7 +91,7 @@ async function assertAuthGuard(baseUrl: string): Promise { expect(wrongLastHasBody).toContain("conditioned page"); const wrongLastMissing = await fetch( - `${baseUrl}/conditioned?role=guest&role=admin&present=yes&blocked=0&blocked=1`, + `${baseUrl}/conditioned?role=guest&role=admin&present=yes&present=&blocked=0&blocked=1`, { headers: conditionedHeaders }, ); const wrongLastMissingBody = await wrongLastMissing.text(); @@ -266,7 +267,9 @@ describe("unsafe middleware matcher rejection", () => { ["/:path(.*)*/end", /may match an empty value or path delimiter/], ["/:path(.*)+/end", /may match an empty value or path delimiter/], ["/:path((?:a+)+)", /contains nested repetition/], - ["/:path((?:a|aa)+)", /contains ambiguous alternatives under unbounded repetition/], + ["/:path((?:a|aa)+)", /contains ambiguous alternatives under repetition/], + ["/:path((?:a|A)+)", /contains ambiguous alternatives under repetition/], + ["/:path((?:a+){10})", /contains nested repetition/], ["/:path(a+.*a+)", /contains overlapping sequential repetition/], ["/:path(a+(?:b*)a+)", /contains overlapping sequential repetition/], ] as const)( diff --git a/tests/shims.test.ts b/tests/shims.test.ts index e865d85f5d..e0db080b95 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7473,19 +7473,30 @@ describe("middleware matcher patterns", () => { expect(matchPattern("/archive/2024-07-10-11", matcher)).toBe(false); }); - it("rejects prefix-ambiguous alternatives only under unbounded repetition", async () => { + it("rejects prefix-ambiguous alternatives under repetition", async () => { const { compileMiddlewareMatcherPattern } = await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); - for (const matcher of ["/:path((?:a|aa)+)", "/:path((?:aa|a)*)", "/:path((?:[a]|aa){1,})"]) { + for (const matcher of [ + "/:path((?:a|aa)+)", + "/:path((?:aa|a)*)", + "/:path((?:[a]|aa){1,})", + "/:path((?:a|aa){2})", + "/:path((?:a|A)+)", + "/:path((?:é|É)+)", + ]) { expect(compileMiddlewareMatcherPattern(matcher), matcher).toMatchObject({ kind: "unsafe", error: expect.stringContaining("ambiguous alternatives"), }); } - expect(compileMiddlewareMatcherPattern("/:path((?:a|aa){2})").regexp).toBeInstanceOf(RegExp); expect(compileMiddlewareMatcherPattern("/:path((?:foo|bar)+)").regexp).toBeInstanceOf(RegExp); + expect(compileMiddlewareMatcherPattern("/:path((?:ab|ac)+)").regexp).toBeInstanceOf(RegExp); + expect(compileMiddlewareMatcherPattern("/:path((?:ab|ac){10})").regexp).toBeInstanceOf(RegExp); + // Non-Unicode ECMAScript ignore-case canonicalization deliberately does + // not fold a non-ASCII character to ASCII. + expect(compileMiddlewareMatcherPattern("/:path((?:k|K)+)").regexp).toBeInstanceOf(RegExp); }); // Ported from Next.js path-to-regexp matcher semantics: @@ -10819,11 +10830,12 @@ describe("isSafeRegex", () => { expect(isSafeRegex("(\\d{2,4})")).toBe(true); }); - it("accepts bounded groups containing bounded repetition", async () => { + it("accepts only fixed-width nested bounded repetition", async () => { const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); expect(isSafeRegex("\\d{4}(?:-\\d{2}){2}")).toBe(true); - expect(isSafeRegex("(?:a{1,2}){3}")).toBe(true); - expect(isSafeRegex("(?:a{1,2}){2,4}")).toBe(true); + expect(isSafeRegex("(?:a{2}){3}")).toBe(true); + expect(isSafeRegex("(?:a{1,2}){3}")).toBe(false); + expect(isSafeRegex("(?:a{1,2}){2,4}")).toBe(false); }); it("rejects nested quantifiers: (a+)+", async () => { @@ -10856,10 +10868,11 @@ describe("isSafeRegex", () => { expect(isSafeRegex("(a+){2,}")).toBe(false); }); - it("accepts bounded repetition around an unbounded inner atom", async () => { + it("rejects bounded outer repetition around an unbounded inner atom", async () => { const { isSafeRegex } = await import("../packages/vinext/src/config/config-matchers.js"); - expect(isSafeRegex("(a+){2}")).toBe(true); - expect(isSafeRegex("(a+){2,4}")).toBe(true); + expect(isSafeRegex("(a+){2}")).toBe(false); + expect(isSafeRegex("(a+){2,4}")).toBe(false); + expect(isSafeRegex("(a+){10}")).toBe(false); }); it("accepts quantifier on group without inner quantifier", async () => { @@ -10931,6 +10944,12 @@ describe("safeRegExp", () => { expect(re).toBeNull(); }); + it("uses the final ignore-case semantics for ambiguous alternatives", async () => { + const { safeRegExp } = await import("../packages/vinext/src/config/config-matchers.js"); + expect(safeRegExp("(?:a|A)+", "i")).toBeNull(); + expect(safeRegExp("(?:ab|ac)+", "i")).toBeInstanceOf(RegExp); + }); + it("returns null for invalid regex syntax", async () => { const { safeRegExp } = await import("../packages/vinext/src/config/config-matchers.js"); const re = safeRegExp("(?P"); @@ -11503,6 +11522,12 @@ describe("checkHasConditions", () => { query: new URLSearchParams("empty="), }), ).toBe(false); + expect( + checkHasConditions([{ type: "query", key: "present", value: "" }], undefined, { + ...ctx, + query: new URLSearchParams("present=yes&present="), + }), + ).toBe(true); }); // -- host conditions -- From 77420b9da2c8bdd26eff2dcd09e3fef9eb60de1a Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:30:36 +0100 Subject: [PATCH 5/9] fix(middleware): bound matcher sequence ambiguity --- packages/vinext/src/utils/regex-safety.ts | 285 ++++++++++++++++-- .../app/mixed/[value]/page.tsx | 3 + .../app/shared/[value]/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 4 +- .../middleware-matcher-redos-child.ts | 6 + tests/middleware-matcher-auth.test.ts | 6 +- tests/shims.test.ts | 37 +++ 7 files changed, 324 insertions(+), 20 deletions(-) create mode 100644 tests/fixtures/middleware-matcher-auth/app/mixed/[value]/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/shared/[value]/page.tsx diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts index 426bd58d52..b67ceda7e3 100644 --- a/packages/vinext/src/utils/regex-safety.ts +++ b/packages/vinext/src/utils/regex-safety.ts @@ -16,11 +16,14 @@ type RegexNode = type RegexSymbol = | { kind: "literal"; key: string; value: string } + | { kind: "class"; key: string; values: ReadonlySet } | { kind: "opaque"; key: string; pattern: string; ignoreCase: boolean }; export type RegexSafetyIssue = | "nested repetition" | "ambiguous alternatives under repetition" + | "ambiguous sequence expansion" + | "overlapping sequential repetition" | "analysis budget exceeded"; const MAX_NODES = 16_384; @@ -29,6 +32,7 @@ const MAX_PATTERN_LENGTH = 65_536; const MAX_WORDS = 4_096; const MAX_WORD_SYMBOLS = 32_768; const MAX_OPAQUE_COMPARISONS = 4_096; +const MAX_SEQUENCE_EXPANSIONS = 256; function canonicalizeIgnoreCase(character: string): string { const upper = character.toUpperCase(); @@ -45,6 +49,40 @@ function literalSymbol(character: string, ignoreCase: boolean): RegexSymbol { return { kind: "literal", key, value: character }; } +function simpleAsciiClassSymbol(raw: string, ignoreCase: boolean): RegexSymbol | null { + const end = raw.length - 1; + if (raw[0] !== "[" || raw[end] !== "]" || raw[1] === "^") return null; + const values = new Set(); + + const add = (character: string): boolean => { + if (character.charCodeAt(0) > 0x7f) return false; + values.add(ignoreCase ? canonicalizeIgnoreCase(character) : character); + return true; + }; + + for (let index = 1; index < end; index++) { + const start = raw[index]; + if (start === "\\") return null; + if (index + 2 < end && raw[index + 1] === "-") { + const rangeEnd = raw[index + 2]; + if (rangeEnd === "\\") return null; + const startCode = start.charCodeAt(0); + const endCode = rangeEnd.charCodeAt(0); + if (startCode > endCode || endCode > 0x7f) return null; + for (let code = startCode; code <= endCode; code++) { + if (!add(String.fromCharCode(code))) return null; + } + index += 2; + } else if (!add(start)) { + return null; + } + } + + if (values.size === 0) return null; + const key = [...values].sort().join(""); + return { kind: "class", key: `class:${key}`, values }; +} + class RegexParser { index = 0; nodes = 0; @@ -182,7 +220,9 @@ class RegexParser { const raw = this.pattern.slice(start, this.index); return this.node({ kind: "atom", - symbol: { kind: "opaque", key: raw, pattern: raw, ignoreCase: this.ignoreCase }, + symbol: + simpleAsciiClassSymbol(raw, this.ignoreCase) ?? + ({ kind: "opaque", key: raw, pattern: raw, ignoreCase: this.ignoreCase } as const), fixedWidth: true, }); } @@ -415,11 +455,12 @@ type TrieEdge = { symbol: RegexSymbol; node: TrieNode }; type TrieNode = { terminal: boolean; literals: Map; + classes: TrieEdge[]; opaque: TrieEdge[]; }; function createTrieNode(): TrieNode { - return { terminal: false, literals: new Map(), opaque: [] }; + return { terminal: false, literals: new Map(), classes: [], opaque: [] }; } function opaqueMatchesLiteral(opaque: RegexSymbol, literal: RegexSymbol): boolean { @@ -431,6 +472,29 @@ function opaqueMatchesLiteral(opaque: RegexSymbol, literal: RegexSymbol): boolea } } +function symbolsMayOverlap(left: RegexSymbol, right: RegexSymbol): boolean { + if (left.kind === "literal" && right.kind === "literal") return left.key === right.key; + if (left.kind === "class" && right.kind === "literal") return left.values.has(right.key); + if (left.kind === "literal" && right.kind === "class") return right.values.has(left.key); + if (left.kind === "class" && right.kind === "class") { + const [smaller, larger] = + left.values.size <= right.values.size + ? [left.values, right.values] + : [right.values, left.values]; + for (const value of smaller) { + if (larger.has(value)) return true; + } + return false; + } + if (left.kind === "opaque" && right.kind === "literal") { + return opaqueMatchesLiteral(left, right); + } + if (left.kind === "literal" && right.kind === "opaque") { + return opaqueMatchesLiteral(right, left); + } + return true; +} + function insertPrefixFreeWord( root: TrieNode, word: RegexSymbol[], @@ -442,6 +506,10 @@ function insertPrefixFreeWord( let edge: TrieEdge | undefined; if (symbol.kind === "literal") { edge = node.literals.get(symbol.key); + for (const classEdge of node.classes) { + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; + if (symbolsMayOverlap(classEdge.symbol, symbol)) return false; + } for (const opaque of node.opaque) { if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; if (opaqueMatchesLiteral(opaque.symbol, symbol)) return false; @@ -450,11 +518,28 @@ function insertPrefixFreeWord( edge = { symbol, node: createTrieNode() }; node.literals.set(symbol.key, edge); } + } else if (symbol.kind === "class") { + for (const literal of node.literals.values()) { + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; + if (symbolsMayOverlap(symbol, literal.symbol)) return false; + } + edge = node.classes.find((candidate) => candidate.symbol.key === symbol.key); + for (const classEdge of node.classes) { + if (classEdge === edge) continue; + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; + if (symbolsMayOverlap(symbol, classEdge.symbol)) return false; + } + if (node.opaque.length > 0) return false; + if (!edge) { + edge = { symbol, node: createTrieNode() }; + node.classes.push(edge); + } } else { for (const literal of node.literals.values()) { if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return false; if (opaqueMatchesLiteral(symbol, literal.symbol)) return false; } + if (node.classes.length > 0) return false; edge = node.opaque.find((candidate) => candidate.symbol.key === symbol.key); if (!edge && node.opaque.length > 0) return false; if (!edge) { @@ -464,15 +549,26 @@ function insertPrefixFreeWord( } node = edge.node; } - if (node.terminal || node.literals.size > 0 || node.opaque.length > 0) return false; + if ( + node.terminal || + node.literals.size > 0 || + node.classes.length > 0 || + node.opaque.length > 0 + ) { + return false; + } node.terminal = true; return true; } -function hasPrefixFreeFiniteLanguage(node: RegexNode): { safe: boolean; budgetExceeded: boolean } { +function hasPrefixFreeFiniteLanguage(node: RegexNode): { + safe: boolean; + budgetExceeded: boolean; + wordCount: number; +} { const budget: WordBudget = { words: 0, symbols: 0, exceeded: false }; const words = fixedWords(node, budget); - if (!words) return { safe: false, budgetExceeded: budget.exceeded }; + if (!words) return { safe: false, budgetExceeded: budget.exceeded, wordCount: 0 }; const root = createTrieNode(); const comparisons = { count: 0 }; for (const word of words) { @@ -480,10 +576,170 @@ function hasPrefixFreeFiniteLanguage(node: RegexNode): { safe: boolean; budgetEx return { safe: false, budgetExceeded: comparisons.count > MAX_OPAQUE_COMPARISONS, + wordCount: words.length, }; } } - return { safe: true, budgetExceeded: false }; + return { safe: true, budgetExceeded: false, wordCount: words.length }; +} + +function ambiguousExpansionFactor(node: RegexNode): number { + switch (node.kind) { + case "atom": + case "assertion": + return 1; + case "alternation": { + const result = hasPrefixFreeFiniteLanguage(node); + if (result.safe) return 1; + if (result.budgetExceeded || result.wordCount === 0) return MAX_SEQUENCE_EXPANSIONS + 1; + return result.wordCount; + } + case "sequence": { + let factor = 1; + for (const child of node.children) { + factor *= ambiguousExpansionFactor(child); + if (factor > MAX_SEQUENCE_EXPANSIONS) return factor; + } + return factor; + } + case "repeat": { + if (node.min !== node.max || !Number.isFinite(node.max)) return 1; + const childFactor = ambiguousExpansionFactor(node.child); + let factor = 1; + for (let count = 0; count < node.max; count++) { + factor *= childFactor; + if (factor > MAX_SEQUENCE_EXPANSIONS) return factor; + } + return factor; + } + } +} + +function isNullable(node: RegexNode): boolean { + switch (node.kind) { + case "atom": + return !node.fixedWidth; + case "assertion": + return true; + case "sequence": + return node.children.every(isNullable); + case "alternation": + return node.branches.some(isNullable); + case "repeat": + return node.min === 0 || isNullable(node.child); + } +} + +function firstSymbols(node: RegexNode): RegexSymbol[] | null { + switch (node.kind) { + case "atom": + return node.symbol ? [node.symbol] : null; + case "assertion": + return []; + case "repeat": + return firstSymbols(node.child); + case "alternation": { + const symbols: RegexSymbol[] = []; + for (const branch of node.branches) { + const branchSymbols = firstSymbols(branch); + if (!branchSymbols) return null; + symbols.push(...branchSymbols); + } + return symbols; + } + case "sequence": { + const symbols: RegexSymbol[] = []; + for (const child of node.children) { + const childSymbols = firstSymbols(child); + if (!childSymbols) return null; + symbols.push(...childSymbols); + if (!isNullable(child)) break; + } + return symbols; + } + } +} + +function lastSymbols(node: RegexNode): RegexSymbol[] | null { + switch (node.kind) { + case "atom": + return node.symbol ? [node.symbol] : null; + case "assertion": + return []; + case "repeat": + return lastSymbols(node.child); + case "alternation": { + const symbols: RegexSymbol[] = []; + for (const branch of node.branches) { + const branchSymbols = lastSymbols(branch); + if (!branchSymbols) return null; + symbols.push(...branchSymbols); + } + return symbols; + } + case "sequence": { + const symbols: RegexSymbol[] = []; + for (let index = node.children.length - 1; index >= 0; index--) { + const child = node.children[index]; + const childSymbols = lastSymbols(child); + if (!childSymbols) return null; + symbols.push(...childSymbols); + if (!isNullable(child)) break; + } + return symbols; + } + } +} + +function boundariesMayOverlap( + left: RegexSymbol[] | null, + right: RegexSymbol[] | null, + comparisons: { count: number }, +): boolean { + if (!left || !right) return true; + for (const leftSymbol of left) { + for (const rightSymbol of right) { + if (++comparisons.count > MAX_OPAQUE_COMPARISONS) return true; + if (symbolsMayOverlap(leftSymbol, rightSymbol)) return true; + } + } + return false; +} + +function findSequenceIssue( + node: Extract, +): RegexSafetyIssue | null { + if (ambiguousExpansionFactor(node) > MAX_SEQUENCE_EXPANSIONS) { + return "ambiguous sequence expansion"; + } + + let pendingRepetitionEnds: Array = []; + const comparisons = { count: 0 }; + let boundaryExpansion = 1; + for (const child of node.children) { + const variableRepetition = + child.kind === "repeat" && (child.min !== child.max || !Number.isFinite(child.max)); + if (variableRepetition) { + const starts = firstSymbols(child); + const overlappingBoundaries = pendingRepetitionEnds.filter((ends) => + boundariesMayOverlap(ends, starts, comparisons), + ).length; + if (overlappingBoundaries > 0) { + boundaryExpansion *= 2 ** overlappingBoundaries; + } else if (!isNullable(child)) { + boundaryExpansion = 1; + } + if (boundaryExpansion > MAX_SEQUENCE_EXPANSIONS) { + return "overlapping sequential repetition"; + } + const ends = lastSymbols(child); + pendingRepetitionEnds = isNullable(child) ? [...pendingRepetitionEnds, ends] : [ends]; + } else if (!isNullable(child)) { + pendingRepetitionEnds = []; + boundaryExpansion = 1; + } + } + return null; } function findSafetyIssue(node: RegexNode): RegexSafetyIssue | null { @@ -492,12 +748,15 @@ function findSafetyIssue(node: RegexNode): RegexSafetyIssue | null { return null; case "assertion": return findSafetyIssue(node.child); - case "sequence": + case "sequence": { + const sequenceIssue = findSequenceIssue(node); + if (sequenceIssue) return sequenceIssue; for (const child of node.children) { const issue = findSafetyIssue(child); if (issue) return issue; } return null; + } case "alternation": for (const branch of node.branches) { const issue = findSafetyIssue(branch); @@ -506,7 +765,6 @@ function findSafetyIssue(node: RegexNode): RegexSafetyIssue | null { return null; case "repeat": { const nestedRepetition = containsConsumingRepetition(node.child); - if (node.max === Infinity && nestedRepetition) return "nested repetition"; if (node.max > 1 && nestedRepetition && exactWidth(node.child) === null) { return "nested repetition"; } @@ -545,14 +803,5 @@ export function regexAtomsMayOverlap(left: string, right: string, ignoreCase = f const leftSymbol = leftWords[0][0]; const rightSymbol = rightWords[0][0]; if (!leftSymbol || !rightSymbol) return true; - if (leftSymbol.kind === "literal" && rightSymbol.kind === "literal") { - return leftSymbol.key === rightSymbol.key; - } - if (leftSymbol.kind === "opaque" && rightSymbol.kind === "literal") { - return opaqueMatchesLiteral(leftSymbol, rightSymbol); - } - if (leftSymbol.kind === "literal" && rightSymbol.kind === "opaque") { - return opaqueMatchesLiteral(rightSymbol, leftSymbol); - } - return true; + return symbolsMayOverlap(leftSymbol, rightSymbol); } diff --git a/tests/fixtures/middleware-matcher-auth/app/mixed/[value]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/mixed/[value]/page.tsx new file mode 100644 index 0000000000..52808b8ec9 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/mixed/[value]/page.tsx @@ -0,0 +1,3 @@ +export default function MixedClassPage() { + return
mixed class secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/shared/[value]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/shared/[value]/page.tsx new file mode 100644 index 0000000000..349f5d47ec --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/shared/[value]/page.tsx @@ -0,0 +1,3 @@ +export default function SharedPrefixPage() { + return
shared prefix secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts index 850a48c886..379c2f0a06 100644 --- a/tests/fixtures/middleware-matcher-auth/middleware.ts +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -17,7 +17,9 @@ export const config = { "/(foo.*|bar)/:path*", "/report{.:ext}", "/archive/:date(\\d{4}(?:-\\d{2}){2})", - "/codes/:value((?:ab|ac)+)", + "/codes/:value((?:[A-Z]{2})+)", + "/shared/:value((?:ab|ac)+)", + "/mixed/:value((?:[a-z]|[0-9])+)", { source: "/conditioned", has: [ diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index 2b37a623f9..b9c5da6479 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -31,6 +31,12 @@ for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { } } +for (const matcher of [`/:path(${"(?:a+)".repeat(10)})`, `/:path(${"(?:a|aa)".repeat(26)})`]) { + if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { + throw new Error(`Unsafe bounded sequence did not fail closed: ${matcher}`); + } +} + // Keep the analysis itself linear for large, disjoint literal alternations. // CJK literals have stable, distinct non-Unicode ignore-case canonical forms. const alternatives = Array.from({ length: 2_000 }, (_, index) => diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 98d1659ddb..042c81c597 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -35,7 +35,9 @@ const PROTECTED_PATHS = [ "/bar/secret", "/report.json", "/archive/2024-07-10", - "/codes/abac", + "/codes/ABCD", + "/shared/abac", + "/mixed/a1z9", ] as const; async function assertAuthGuard(baseUrl: string): Promise { @@ -270,6 +272,8 @@ describe("unsafe middleware matcher rejection", () => { ["/:path((?:a|aa)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a|A)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a+){10})", /contains nested repetition/], + [`/:path(${"(?:a+)".repeat(10)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a|aa)".repeat(26)})`, /contains ambiguous sequence expansion/], ["/:path(a+.*a+)", /contains overlapping sequential repetition/], ["/:path(a+(?:b*)a+)", /contains overlapping sequential repetition/], ] as const)( diff --git a/tests/shims.test.ts b/tests/shims.test.ts index e0db080b95..3015907a19 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7499,6 +7499,42 @@ describe("middleware matcher patterns", () => { expect(compileMiddlewareMatcherPattern("/:path((?:k|K)+)").regexp).toBeInstanceOf(RegExp); }); + it("handles fixed-width nested repeats and simple class intersections", async () => { + const { compileMiddlewareMatcherPattern } = + await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); + + for (const matcher of ["/codes/:value((?:[A-Z]{2})+)", "/mixed/:value((?:[a-z]|[0-9])+)"]) { + expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); + } + + for (const matcher of [ + "/:path((?:[a-z]|[A-Z])+)", + "/:path((?:[a-f]|[d-z])+)", + "/:path((?:[a-z]|m)+)", + ]) { + expect(compileMiddlewareMatcherPattern(matcher), matcher).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("ambiguous alternatives"), + }); + } + }); + + it("rejects bounded sequence-level ambiguity within the analysis budget", async () => { + const { compileMiddlewareMatcherPattern } = + await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); + const adjacentRepetitions = `/:path(${"(?:a+)".repeat(10)})`; + const expandingChoices = `/:path(${"(?:a|aa)".repeat(26)})`; + + expect(compileMiddlewareMatcherPattern(adjacentRepetitions)).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("overlapping sequential repetition"), + }); + expect(compileMiddlewareMatcherPattern(expandingChoices)).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("ambiguous sequence expansion"), + }); + }); + // Ported from Next.js path-to-regexp matcher semantics: // packages/next/src/lib/try-to-parse-path.ts // https://github.com/vercel/next.js/blob/canary/packages/next/src/lib/try-to-parse-path.ts @@ -10881,6 +10917,7 @@ describe("isSafeRegex", () => { expect(isSafeRegex("(ab)+")).toBe(true); expect(isSafeRegex("(foo|bar)*")).toBe(true); expect(isSafeRegex("(?:(?!\\.)[^/])+?")).toBe(true); + expect(isSafeRegex("[^/]+.*")).toBe(true); }); it("treats escaped characters as safe", async () => { From 3391308e299fd711f1feace15fefc58183c039ba Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:38:58 +0100 Subject: [PATCH 6/9] fix(middleware): model matcher shorthand classes --- packages/vinext/src/utils/regex-safety.ts | 112 +++++++++++++++--- .../app/bracket-shorthand/[value]/page.tsx | 3 + .../app/shorthand/[value]/page.tsx | 3 + .../middleware-matcher-auth/middleware.ts | 2 + .../middleware-matcher-redos-child.ts | 5 +- tests/middleware-matcher-auth.test.ts | 4 + tests/shims.test.ts | 27 ++++- 7 files changed, 135 insertions(+), 21 deletions(-) create mode 100644 tests/fixtures/middleware-matcher-auth/app/bracket-shorthand/[value]/page.tsx create mode 100644 tests/fixtures/middleware-matcher-auth/app/shorthand/[value]/page.tsx diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts index b67ceda7e3..54ed682eb1 100644 --- a/packages/vinext/src/utils/regex-safety.ts +++ b/packages/vinext/src/utils/regex-safety.ts @@ -16,9 +16,16 @@ type RegexNode = type RegexSymbol = | { kind: "literal"; key: string; value: string } - | { kind: "class"; key: string; values: ReadonlySet } + | { + kind: "class"; + key: string; + values: ReadonlySet; + nonAscii: NonAsciiDomain; + } | { kind: "opaque"; key: string; pattern: string; ignoreCase: boolean }; +type NonAsciiDomain = "none" | "whitespace" | "non-whitespace" | "all"; + export type RegexSafetyIssue = | "nested repetition" | "ambiguous alternatives under repetition" @@ -33,6 +40,7 @@ const MAX_WORDS = 4_096; const MAX_WORD_SYMBOLS = 32_768; const MAX_OPAQUE_COMPARISONS = 4_096; const MAX_SEQUENCE_EXPANSIONS = 256; +const MAX_OVERLAPPING_VARIABLE_BOUNDARIES = 7; function canonicalizeIgnoreCase(character: string): string { const upper = character.toUpperCase(); @@ -49,10 +57,43 @@ function literalSymbol(character: string, ignoreCase: boolean): RegexSymbol { return { kind: "literal", key, value: character }; } -function simpleAsciiClassSymbol(raw: string, ignoreCase: boolean): RegexSymbol | null { +function createClassSymbol(values: ReadonlySet, nonAscii: NonAsciiDomain): RegexSymbol { + const key = [...values].sort().join(""); + return { kind: "class", key: `class:${nonAscii}:${key}`, values, nonAscii }; +} + +function shorthandClassSymbol(shorthand: string, ignoreCase: boolean): RegexSymbol | null { + if (!"dDwWsS".includes(shorthand)) return null; + const regexp = new RegExp(`\\${shorthand}`); + const values = new Set(); + for (let code = 0; code <= 0x7f; code++) { + const character = String.fromCharCode(code); + if (regexp.test(character)) { + values.add(ignoreCase ? canonicalizeIgnoreCase(character) : character); + } + } + const nonAscii: NonAsciiDomain = + shorthand === "d" || shorthand === "w" + ? "none" + : shorthand === "s" + ? "whitespace" + : shorthand === "S" + ? "non-whitespace" + : "all"; + return createClassSymbol(values, nonAscii); +} + +function unionNonAscii(left: NonAsciiDomain, right: NonAsciiDomain): NonAsciiDomain { + if (left === "none") return right; + if (right === "none" || left === right) return left; + return "all"; +} + +function simpleClassSymbol(raw: string, ignoreCase: boolean): RegexSymbol | null { const end = raw.length - 1; if (raw[0] !== "[" || raw[end] !== "]" || raw[1] === "^") return null; const values = new Set(); + let nonAscii: NonAsciiDomain = "none"; const add = (character: string): boolean => { if (character.charCodeAt(0) > 0x7f) return false; @@ -60,9 +101,28 @@ function simpleAsciiClassSymbol(raw: string, ignoreCase: boolean): RegexSymbol | return true; }; + const addClass = (symbol: RegexSymbol): boolean => { + if (symbol.kind !== "class") return false; + for (const value of symbol.values) values.add(value); + nonAscii = unionNonAscii(nonAscii, symbol.nonAscii); + return true; + }; + for (let index = 1; index < end; index++) { const start = raw[index]; - if (start === "\\") return null; + if (start === "\\") { + const escaped = raw[++index]; + if (escaped === undefined) return null; + const shorthand = shorthandClassSymbol(escaped, ignoreCase); + if (shorthand) { + if (!addClass(shorthand)) return null; + } else if ("\\-]".includes(escaped)) { + if (!add(escaped)) return null; + } else { + return null; + } + continue; + } if (index + 2 < end && raw[index + 1] === "-") { const rangeEnd = raw[index + 2]; if (rangeEnd === "\\") return null; @@ -78,9 +138,8 @@ function simpleAsciiClassSymbol(raw: string, ignoreCase: boolean): RegexSymbol | } } - if (values.size === 0) return null; - const key = [...values].sort().join(""); - return { kind: "class", key: `class:${key}`, values }; + if (values.size === 0 && nonAscii === "none") return null; + return createClassSymbol(values, nonAscii); } class RegexParser { @@ -221,7 +280,7 @@ class RegexParser { return this.node({ kind: "atom", symbol: - simpleAsciiClassSymbol(raw, this.ignoreCase) ?? + simpleClassSymbol(raw, this.ignoreCase) ?? ({ kind: "opaque", key: raw, pattern: raw, ignoreCase: this.ignoreCase } as const), fixedWidth: true, }); @@ -235,6 +294,10 @@ class RegexParser { if (escaped === "b" || escaped === "B") { return this.node({ kind: "assertion", child: this.node({ kind: "sequence", children: [] }) }); } + const shorthand = shorthandClassSymbol(escaped, this.ignoreCase); + if (shorthand) { + return this.node({ kind: "atom", symbol: shorthand, fixedWidth: true }); + } if (/\d/.test(escaped)) { return this.node({ kind: "atom", symbol: null, fixedWidth: false }); } @@ -472,10 +535,26 @@ function opaqueMatchesLiteral(opaque: RegexSymbol, literal: RegexSymbol): boolea } } +function classMatchesLiteral(characterClass: RegexSymbol, literal: RegexSymbol): boolean { + if (characterClass.kind !== "class" || literal.kind !== "literal") return false; + if (literal.value.charCodeAt(0) <= 0x7f) return characterClass.values.has(literal.key); + if (characterClass.nonAscii === "all") return true; + const whitespace = /\s/.test(literal.value); + return whitespace + ? characterClass.nonAscii === "whitespace" + : characterClass.nonAscii === "non-whitespace"; +} + +function nonAsciiDomainsOverlap(left: NonAsciiDomain, right: NonAsciiDomain): boolean { + if (left === "none" || right === "none") return false; + if (left === "all" || right === "all") return true; + return left === right; +} + function symbolsMayOverlap(left: RegexSymbol, right: RegexSymbol): boolean { if (left.kind === "literal" && right.kind === "literal") return left.key === right.key; - if (left.kind === "class" && right.kind === "literal") return left.values.has(right.key); - if (left.kind === "literal" && right.kind === "class") return right.values.has(left.key); + if (left.kind === "class" && right.kind === "literal") return classMatchesLiteral(left, right); + if (left.kind === "literal" && right.kind === "class") return classMatchesLiteral(right, left); if (left.kind === "class" && right.kind === "class") { const [smaller, larger] = left.values.size <= right.values.size @@ -484,7 +563,7 @@ function symbolsMayOverlap(left: RegexSymbol, right: RegexSymbol): boolean { for (const value of smaller) { if (larger.has(value)) return true; } - return false; + return nonAsciiDomainsOverlap(left.nonAscii, right.nonAscii); } if (left.kind === "opaque" && right.kind === "literal") { return opaqueMatchesLiteral(left, right); @@ -715,7 +794,7 @@ function findSequenceIssue( let pendingRepetitionEnds: Array = []; const comparisons = { count: 0 }; - let boundaryExpansion = 1; + let overlappingBoundaryCount = 0; for (const child of node.children) { const variableRepetition = child.kind === "repeat" && (child.min !== child.max || !Number.isFinite(child.max)); @@ -725,18 +804,21 @@ function findSequenceIssue( boundariesMayOverlap(ends, starts, comparisons), ).length; if (overlappingBoundaries > 0) { - boundaryExpansion *= 2 ** overlappingBoundaries; + overlappingBoundaryCount++; } else if (!isNullable(child)) { - boundaryExpansion = 1; + overlappingBoundaryCount = 0; } - if (boundaryExpansion > MAX_SEQUENCE_EXPANSIONS) { + // Seven overlapping boundaries already create a degree-seven partition + // search over the pathname length. Reject that chain before compiling a + // request-facing expression; shorter common pairs remain valid. + if (overlappingBoundaryCount >= MAX_OVERLAPPING_VARIABLE_BOUNDARIES) { return "overlapping sequential repetition"; } const ends = lastSymbols(child); pendingRepetitionEnds = isNullable(child) ? [...pendingRepetitionEnds, ends] : [ends]; } else if (!isNullable(child)) { pendingRepetitionEnds = []; - boundaryExpansion = 1; + overlappingBoundaryCount = 0; } } return null; diff --git a/tests/fixtures/middleware-matcher-auth/app/bracket-shorthand/[value]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/bracket-shorthand/[value]/page.tsx new file mode 100644 index 0000000000..27059eeb83 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/bracket-shorthand/[value]/page.tsx @@ -0,0 +1,3 @@ +export default function BracketShorthandPage() { + return
bracket shorthand secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/app/shorthand/[value]/page.tsx b/tests/fixtures/middleware-matcher-auth/app/shorthand/[value]/page.tsx new file mode 100644 index 0000000000..781f7620e4 --- /dev/null +++ b/tests/fixtures/middleware-matcher-auth/app/shorthand/[value]/page.tsx @@ -0,0 +1,3 @@ +export default function ShorthandPage() { + return
shorthand secret
; +} diff --git a/tests/fixtures/middleware-matcher-auth/middleware.ts b/tests/fixtures/middleware-matcher-auth/middleware.ts index 379c2f0a06..752fdfcf80 100644 --- a/tests/fixtures/middleware-matcher-auth/middleware.ts +++ b/tests/fixtures/middleware-matcher-auth/middleware.ts @@ -20,6 +20,8 @@ export const config = { "/codes/:value((?:[A-Z]{2})+)", "/shared/:value((?:ab|ac)+)", "/mixed/:value((?:[a-z]|[0-9])+)", + "/shorthand/:value((?:\\d|[a-z])+)", + "/bracket-shorthand/:value((?:[\\d]|[a-z])+)", { source: "/conditioned", has: [ diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index b9c5da6479..af4d3b6e32 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -31,7 +31,10 @@ for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { } } -for (const matcher of [`/:path(${"(?:a+)".repeat(10)})`, `/:path(${"(?:a|aa)".repeat(26)})`]) { +for (const matcher of [ + ...[8, 9, 10].map((count) => `/:path(${"(?:a+)".repeat(count)})`), + `/:path(${"(?:a|aa)".repeat(26)})`, +]) { if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { throw new Error(`Unsafe bounded sequence did not fail closed: ${matcher}`); } diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 042c81c597..9a8c8cf9a7 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -38,6 +38,8 @@ const PROTECTED_PATHS = [ "/codes/ABCD", "/shared/abac", "/mixed/a1z9", + "/shorthand/a1z9", + "/bracket-shorthand/a1z9", ] as const; async function assertAuthGuard(baseUrl: string): Promise { @@ -272,6 +274,8 @@ describe("unsafe middleware matcher rejection", () => { ["/:path((?:a|aa)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a|A)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a+){10})", /contains nested repetition/], + [`/:path(${"(?:a+)".repeat(8)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+)".repeat(9)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(10)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a|aa)".repeat(26)})`, /contains ambiguous sequence expansion/], ["/:path(a+.*a+)", /contains overlapping sequential repetition/], diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 3015907a19..3eba6ca3e1 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7507,10 +7507,22 @@ describe("middleware matcher patterns", () => { expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); } + for (const matcher of [ + "/shorthand/:value((?:\\d|[a-z])+)", + "/bracket-shorthand/:value((?:[\\d]|[a-z])+)", + "/space-or-word/:value((?:\\s|[a-z])+)", + "/digit-or-not/:value((?:\\d|\\D)+)", + ]) { + expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); + } + for (const matcher of [ "/:path((?:[a-z]|[A-Z])+)", "/:path((?:[a-f]|[d-z])+)", "/:path((?:[a-z]|m)+)", + "/:path((?:\\w|[a-z])+)", + "/:path((?:[\\w]|[A-Z])+)", + "/:path((?:\\D|[a-z])+)", ]) { expect(compileMiddlewareMatcherPattern(matcher), matcher).toMatchObject({ kind: "unsafe", @@ -7522,13 +7534,18 @@ describe("middleware matcher patterns", () => { it("rejects bounded sequence-level ambiguity within the analysis budget", async () => { const { compileMiddlewareMatcherPattern } = await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); - const adjacentRepetitions = `/:path(${"(?:a+)".repeat(10)})`; const expandingChoices = `/:path(${"(?:a|aa)".repeat(26)})`; - expect(compileMiddlewareMatcherPattern(adjacentRepetitions)).toMatchObject({ - kind: "unsafe", - error: expect.stringContaining("overlapping sequential repetition"), - }); + for (const count of [8, 9, 10]) { + const adjacentRepetitions = `/:path(${"(?:a+)".repeat(count)})`; + expect( + compileMiddlewareMatcherPattern(adjacentRepetitions), + `${count} repeats`, + ).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("overlapping sequential repetition"), + }); + } expect(compileMiddlewareMatcherPattern(expandingChoices)).toMatchObject({ kind: "unsafe", error: expect.stringContaining("ambiguous sequence expansion"), From 100de95d1a872937130fcaea1ca1b1dc973181ef Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:44:24 +0100 Subject: [PATCH 7/9] fix(middleware): reject quadratic matcher sequences --- packages/vinext/src/utils/regex-safety.ts | 11 ++++++----- tests/fixtures/middleware-matcher-redos-child.ts | 15 ++++++++++++++- tests/middleware-matcher-auth.test.ts | 2 ++ tests/shims.test.ts | 4 +++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts index 54ed682eb1..1169b91d14 100644 --- a/packages/vinext/src/utils/regex-safety.ts +++ b/packages/vinext/src/utils/regex-safety.ts @@ -40,7 +40,7 @@ const MAX_WORDS = 4_096; const MAX_WORD_SYMBOLS = 32_768; const MAX_OPAQUE_COMPARISONS = 4_096; const MAX_SEQUENCE_EXPANSIONS = 256; -const MAX_OVERLAPPING_VARIABLE_BOUNDARIES = 7; +const MAX_SAFE_OVERLAPPING_VARIABLE_BOUNDARIES = 1; function canonicalizeIgnoreCase(character: string): string { const upper = character.toUpperCase(); @@ -808,10 +808,11 @@ function findSequenceIssue( } else if (!isNullable(child)) { overlappingBoundaryCount = 0; } - // Seven overlapping boundaries already create a degree-seven partition - // search over the pathname length. Reject that chain before compiling a - // request-facing expression; shorter common pairs remain valid. - if (overlappingBoundaryCount >= MAX_OVERLAPPING_VARIABLE_BOUNDARIES) { + // One overlapping boundary has linearly many partitions over the input + // length. A second makes that search quadratic, and every additional + // boundary raises the degree again. Preserve common two-repeat patterns, + // but reject longer overlapping chains before compiling them. + if (overlappingBoundaryCount > MAX_SAFE_OVERLAPPING_VARIABLE_BOUNDARIES) { return "overlapping sequential repetition"; } const ends = lastSymbols(child); diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index af4d3b6e32..ee6bd33f70 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -32,7 +32,7 @@ for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { } for (const matcher of [ - ...[8, 9, 10].map((count) => `/:path(${"(?:a+)".repeat(count)})`), + ...[6, 7, 8, 9, 10].map((count) => `/:path(${"(?:a+)".repeat(count)})`), `/:path(${"(?:a|aa)".repeat(26)})`, ]) { if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { @@ -40,6 +40,19 @@ for (const matcher of [ } } +// A single overlapping boundary is kept for common two-repeat matchers, and +// non-overlapping repetitions do not compound the partition search. +for (const pattern of ["(?:a+)(?:a+)", "(?:a+)(?:b+)(?:a+)", "[^/]+.*"]) { + const issue = analyzeRegexSafety(pattern, { ignoreCase: true }); + if (issue) throw new Error(`Safe sequence was rejected: ${pattern} (${issue})`); +} +if (!matchPattern(`/${"a".repeat(3_000)}`, "/:path((?:a+)(?:a+))")) { + throw new Error("Safe two-repeat matcher did not match"); +} +if (matchPattern(`/${"a".repeat(3_000)}b`, "/:path((?:a+)(?:a+))")) { + throw new Error("Safe two-repeat matcher matched a near miss"); +} + // Keep the analysis itself linear for large, disjoint literal alternations. // CJK literals have stable, distinct non-Unicode ignore-case canonical forms. const alternatives = Array.from({ length: 2_000 }, (_, index) => diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 9a8c8cf9a7..48d9afc7a6 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -274,6 +274,8 @@ describe("unsafe middleware matcher rejection", () => { ["/:path((?:a|aa)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a|A)+)", /contains ambiguous alternatives under repetition/], ["/:path((?:a+){10})", /contains nested repetition/], + [`/:path(${"(?:a+)".repeat(6)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+)".repeat(7)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(8)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(9)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(10)})`, /contains overlapping sequential repetition/], diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 3eba6ca3e1..574d1bcd7b 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7512,6 +7512,8 @@ describe("middleware matcher patterns", () => { "/bracket-shorthand/:value((?:[\\d]|[a-z])+)", "/space-or-word/:value((?:\\s|[a-z])+)", "/digit-or-not/:value((?:\\d|\\D)+)", + "/two-repeats/:value((?:a+)(?:a+))", + "/disjoint-repeats/:value((?:a+)(?:b+)(?:a+))", ]) { expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); } @@ -7536,7 +7538,7 @@ describe("middleware matcher patterns", () => { await import("../packages/vinext/src/server/middleware-matcher-pattern.js"); const expandingChoices = `/:path(${"(?:a|aa)".repeat(26)})`; - for (const count of [8, 9, 10]) { + for (const count of [3, 6, 7, 8, 9, 10]) { const adjacentRepetitions = `/:path(${"(?:a+)".repeat(count)})`; expect( compileMiddlewareMatcherPattern(adjacentRepetitions), From 3618500e8ce492a3f29a778cc9ebe663a313453f Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 00:50:25 +0100 Subject: [PATCH 8/9] fix(middleware): flatten matcher sequence wrappers --- packages/vinext/src/utils/regex-safety.ts | 7 ++++++- tests/fixtures/middleware-matcher-redos-child.ts | 9 ++++++++- tests/middleware-matcher-auth.test.ts | 3 +++ tests/shims.test.ts | 12 ++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts index 1169b91d14..c4affed833 100644 --- a/packages/vinext/src/utils/regex-safety.ts +++ b/packages/vinext/src/utils/regex-safety.ts @@ -177,7 +177,12 @@ class RegexParser { while (this.index < this.pattern.length) { const character = this.pattern[this.index]; if (character === "|" || character === ")") break; - children.push(this.parseTerm()); + const term = this.parseTerm(); + // Parentheses around a sequence do not change which terms can consume + // adjacent input. Flatten those transparent wrappers so empty groups + // cannot hide overlapping repetitions from sequence analysis. + if (term.kind === "sequence") children.push(...term.children); + else children.push(term); } return children.length === 1 ? children[0] : this.node({ kind: "sequence", children }); } diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index ee6bd33f70..d6c2d72779 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -33,6 +33,7 @@ for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { for (const matcher of [ ...[6, 7, 8, 9, 10].map((count) => `/:path(${"(?:a+)".repeat(count)})`), + ...[6, 7, 8].map((count) => `/:path(${"(?:a+(?:))".repeat(count)})`), `/:path(${"(?:a|aa)".repeat(26)})`, ]) { if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { @@ -42,7 +43,13 @@ for (const matcher of [ // A single overlapping boundary is kept for common two-repeat matchers, and // non-overlapping repetitions do not compound the partition search. -for (const pattern of ["(?:a+)(?:a+)", "(?:a+)(?:b+)(?:a+)", "[^/]+.*"]) { +for (const pattern of [ + "(?:a+)(?:a+)", + "(?:a+(?:))(?:a+(?:))", + "(?:a+)(?:b+)(?:a+)", + "(?:a+(?:))(?:b+(?:))(?:a+(?:))", + "[^/]+.*", +]) { const issue = analyzeRegexSafety(pattern, { ignoreCase: true }); if (issue) throw new Error(`Safe sequence was rejected: ${pattern} (${issue})`); } diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 48d9afc7a6..36d68f0332 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -276,6 +276,9 @@ describe("unsafe middleware matcher rejection", () => { ["/:path((?:a+){10})", /contains nested repetition/], [`/:path(${"(?:a+)".repeat(6)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(7)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+(?:))".repeat(6)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+(?:))".repeat(7)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+(?:))".repeat(8)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(8)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(9)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(10)})`, /contains overlapping sequential repetition/], diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 574d1bcd7b..4223b0a6f1 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7513,7 +7513,9 @@ describe("middleware matcher patterns", () => { "/space-or-word/:value((?:\\s|[a-z])+)", "/digit-or-not/:value((?:\\d|\\D)+)", "/two-repeats/:value((?:a+)(?:a+))", + "/wrapped-two-repeats/:value((?:a+(?:))(?:a+(?:)))", "/disjoint-repeats/:value((?:a+)(?:b+)(?:a+))", + "/wrapped-disjoint-repeats/:value((?:a+(?:))(?:b+(?:))(?:a+(?:)))", ]) { expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); } @@ -7548,6 +7550,16 @@ describe("middleware matcher patterns", () => { error: expect.stringContaining("overlapping sequential repetition"), }); } + for (const count of [3, 6, 7, 8]) { + const wrappedRepetitions = `/:path(${"(?:a+(?:))".repeat(count)})`; + expect( + compileMiddlewareMatcherPattern(wrappedRepetitions), + `${count} wrapped repeats`, + ).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("overlapping sequential repetition"), + }); + } expect(compileMiddlewareMatcherPattern(expandingChoices)).toMatchObject({ kind: "unsafe", error: expect.stringContaining("ambiguous sequence expansion"), From 95d62e5f5974a916116027b88abdd2bfc6927bb3 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 11 Jul 2026 02:16:08 +0100 Subject: [PATCH 9/9] fix(middleware): unwrap exact-one matcher repeats --- packages/vinext/src/utils/regex-safety.ts | 4 ++++ tests/fixtures/middleware-matcher-redos-child.ts | 7 +++++++ tests/middleware-matcher-auth.test.ts | 4 ++++ tests/shims.test.ts | 9 +++++++++ 4 files changed, 24 insertions(+) diff --git a/packages/vinext/src/utils/regex-safety.ts b/packages/vinext/src/utils/regex-safety.ts index c4affed833..9f35d700bc 100644 --- a/packages/vinext/src/utils/regex-safety.ts +++ b/packages/vinext/src/utils/regex-safety.ts @@ -192,6 +192,10 @@ class RegexParser { const quantifier = this.parseQuantifier(); if (!quantifier) return atom; if (this.pattern[this.index] === "?") this.index++; + // An exact-one quantifier does not change the language or consumption of + // its child. Remove it so it cannot hide a variable repetition from the + // surrounding sequence analysis. + if (quantifier.min === 1 && quantifier.max === 1) return atom; return this.node({ kind: "repeat", child: atom, ...quantifier }); } diff --git a/tests/fixtures/middleware-matcher-redos-child.ts b/tests/fixtures/middleware-matcher-redos-child.ts index d6c2d72779..232862e809 100644 --- a/tests/fixtures/middleware-matcher-redos-child.ts +++ b/tests/fixtures/middleware-matcher-redos-child.ts @@ -34,6 +34,10 @@ for (const matcher of ["/:path((?:a+){10})", "/:path((?:a|A)+)"]) { for (const matcher of [ ...[6, 7, 8, 9, 10].map((count) => `/:path(${"(?:a+)".repeat(count)})`), ...[6, 7, 8].map((count) => `/:path(${"(?:a+(?:))".repeat(count)})`), + ...[6, 8].map((count) => `/:path(${"(?:a+){1}".repeat(count)})`), + ...[6, 8].map((count) => `/:path(${"(?:a+){1,1}".repeat(count)})`), + `/:path(${"(?:a+){1}?".repeat(6)})`, + ...[6, 8].map((count) => `/:path(${"(?:(?:a+){1}){1,1}".repeat(count)})`), `/:path(${"(?:a|aa)".repeat(26)})`, ]) { if (!matchPattern(`/${"a".repeat(3_000)}b`, matcher)) { @@ -46,8 +50,11 @@ for (const matcher of [ for (const pattern of [ "(?:a+)(?:a+)", "(?:a+(?:))(?:a+(?:))", + "(?:a+){1}(?:a+){1,1}", + "(?:a+){1}?(?:a+){1}", "(?:a+)(?:b+)(?:a+)", "(?:a+(?:))(?:b+(?:))(?:a+(?:))", + "(?:(?:a+){1}){1,1}(?:b+){1}(?:(?:a+){1,1}){1}", "[^/]+.*", ]) { const issue = analyzeRegexSafety(pattern, { ignoreCase: true }); diff --git a/tests/middleware-matcher-auth.test.ts b/tests/middleware-matcher-auth.test.ts index 36d68f0332..dc51987b52 100644 --- a/tests/middleware-matcher-auth.test.ts +++ b/tests/middleware-matcher-auth.test.ts @@ -279,6 +279,10 @@ describe("unsafe middleware matcher rejection", () => { [`/:path(${"(?:a+(?:))".repeat(6)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+(?:))".repeat(7)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+(?:))".repeat(8)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+){1}".repeat(6)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+){1}".repeat(8)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:a+){1,1}".repeat(6)})`, /contains overlapping sequential repetition/], + [`/:path(${"(?:(?:a+){1}){1,1}".repeat(6)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(8)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(9)})`, /contains overlapping sequential repetition/], [`/:path(${"(?:a+)".repeat(10)})`, /contains overlapping sequential repetition/], diff --git a/tests/shims.test.ts b/tests/shims.test.ts index 4223b0a6f1..c3128cdc7e 100644 --- a/tests/shims.test.ts +++ b/tests/shims.test.ts @@ -7514,8 +7514,10 @@ describe("middleware matcher patterns", () => { "/digit-or-not/:value((?:\\d|\\D)+)", "/two-repeats/:value((?:a+)(?:a+))", "/wrapped-two-repeats/:value((?:a+(?:))(?:a+(?:)))", + "/exact-one-two-repeats/:value((?:a+){1}(?:a+){1,1})", "/disjoint-repeats/:value((?:a+)(?:b+)(?:a+))", "/wrapped-disjoint-repeats/:value((?:a+(?:))(?:b+(?:))(?:a+(?:)))", + "/exact-one-disjoint-repeats/:value((?:(?:a+){1}){1,1}(?:b+){1}(?:(?:a+){1,1}){1})", ]) { expect(compileMiddlewareMatcherPattern(matcher).regexp, matcher).toBeInstanceOf(RegExp); } @@ -7560,6 +7562,13 @@ describe("middleware matcher patterns", () => { error: expect.stringContaining("overlapping sequential repetition"), }); } + for (const wrapper of ["(?:a+){1}", "(?:a+){1,1}", "(?:a+){1}?", "(?:(?:a+){1}){1,1}"]) { + const exactOneRepetitions = `/:path(${wrapper.repeat(6)})`; + expect(compileMiddlewareMatcherPattern(exactOneRepetitions), wrapper).toMatchObject({ + kind: "unsafe", + error: expect.stringContaining("overlapping sequential repetition"), + }); + } expect(compileMiddlewareMatcherPattern(expandingChoices)).toMatchObject({ kind: "unsafe", error: expect.stringContaining("ambiguous sequence expansion"),