fix: treat a leading ? in an extglob body as a wildcard

[maximilliangrand]

Problem

An extglob whose body — or whose first alternative — begins with ? is not parsed as an extglob at all.

const pm = require('picomatch'); // 4.0.5

pm.makeRe('@(?)')             //=> /^(?:@(\?))$/
pm.makeRe('*(?)')             //=> /^(?:(?!\.)(?=.)[^/]*?(\?))$/
pm.makeRe('@(?ar|zip)')       //=> /^(?:@(\?ar|zip))$/

pm.isMatch('a', '@(?)')                    //=> false
pm.isMatch('bb', '*(?)')                   //=> false
pm.isMatch('abc', '+(?)')                  //=> false
pm.isMatch('a', '?(?)')                    //=> false
pm.isMatch('file.jar', 'file.@(?ar|zip)')  //=> false
pm.isMatch('a', '!(?)')                    //=> true

The @ is emitted literally and the ? is escaped, so @(?ar|zip) compiles to a pattern that wants a literal @?ar. For reference, bash 3.2 with shopt -s extglob:

$ shopt -s extglob
$ [[ a == @(?) ]]                      && echo match   # match
$ [[ ab == @(?) ]]                     && echo match   # no output
$ [[ bb == *(?) ]]                     && echo match   # match
$ [[ abc == +(?) ]]                    && echo match   # match
$ [[ a == ?(?) ]]                      && echo match   # match
$ [[ ba == @(?a|b) ]]                  && echo match   # match
$ [[ file.jar == file.@(?ar|zip) ]]    && echo match   # match
$ [[ a == !(?) ]]                      && echo match   # no output

minimatch@10.2.6 agrees with bash on the @(, *(, +( and ?( cases above. It does not agree on the last line — minimatch('a', '!(?)') returns true where bash does not match. That one matters here, so I call it out rather than claiming across-the-board agreement; see the assertion section below.

picomatch@2.3.1 produces byte-identical wrong regexes to 4.0.5, so this is long-standing rather than a recent regression.

Root cause

Two coupled places in lib/parse.js.

1. Extglob detection. Four openers use a blanket "the character after ( is not ?" guard:

// lib/parse.js:1023  `?`
if (!isGroup && opts.noextglob !== true && peek() === '(' && peek(2) !== '?') {
// lib/parse.js:1072  `+`
if (opts.noextglob !== true && peek() === '(' && peek(2) !== '?') {
// lib/parse.js:1096  `@`
if (opts.noextglob !== true && peek() === '(' && peek(2) !== '?') {
// lib/parse.js:1140  `*`
if (opts.noextglob !== true && /^\([^?]/.test(rest)) {

The guard is there so picomatch does not swallow a raw regex group — (?:, (?=, (?!, (?< — which is a documented feature (test/regex-features.js:20 pins foo/*(?<!d)baz). But it also rejects a plain ? wildcard.

The ! handler at lib/parse.js:1054 already gets this right:

if (opts.noextglob !== true && peek() === '(') {
  if (peek(2) !== '?' || !/[!=<:]/.test(peek(3))) {
    extglobOpen('negate', value);

2. Escaping inside the body. lib/parse.js:1028 escapes any ? whose previous token is a paren, because (? is an invalid quantifier position in a JS regex:

if (prev && prev.type === 'paren') {
  const next = peek();
  let output = value;
  if ((prev.value === '(' && !/[!=<:]/.test(next)) || …) {
    output = `\\${value}`;
  }

Correct for a plain (, wrong for an extglob's (, which opens an alternation body where ? is a wildcard. Note that * already takes the wildcard path here — which is exactly why c!(*)z behaves correctly on master today while c!(?)z does not.

The fix

Two helpers next to the existing tokenizing helpers, then reused:

const isExtglobOpen = () => {
  if (opts.noextglob === true || peek() !== '(') return false;
  if (peek(2) !== '?') return true;
  return peek(3) !== undefined && !/[!=<:]/.test(peek(3));
};

const afterExtglobOpenParen = () => {
  return prev.value === '(' && (prev.extglob === true || (prev.prev && prev.prev.type === 'at'));
};

isExtglobOpen() is the ! handler's condition plus an end-of-input guard; it now replaces the blanket guard in the ?, +, @ and * handlers, and the ! handler collapses onto it. afterExtglobOpenParen() gates the escaping branch so it applies only to plain regex parens.

@( needed a small allowance: unlike the other four it is tokenized as an at token followed by a plain paren, so it is recognized through the token before the paren rather than by adding a flag to the paren token — the public parse() token shape is unchanged.

Deliberately preserved:

• noextglob: true still disables extglobs — the option check lives inside the helper.
• (?:, (?=, (?!, (?< still open regex groups. makeRe('@(?:a|b)').source === '^(?:@(?:a|b))$' and foo/*(?<!d)baz are pinned by tests.
• Malformed input stays byte-identical for *(, *(), a*(, @(, +(, ?(, !(, *(a, *), and — thanks to the peek(3) !== undefined guard — for the trailing @(?, *(?, +(? and ?(? as well. The one malformed pattern that does change is !(?: makeRe('!(?').source goes from $^ to ^(?!^(?:\(\?)$).*$, i.e. from "matches nothing" to picomatch's ordinary leading-! negation of the literal (?. Both are artifacts of an unterminated extglob; I did not consider either worth preserving over the other, but it is a change.

Safety: this touches the repeated-extglob ReDoS guard, deliberately

This is not a security fix and I am not claiming a vulnerability, but it does interact with the open ReDoS work (#175, mitigated in #182), so I want to be precise instead of hand-waving.

Parsing ?-leading bodies as extglobs makes more patterns reach the +(...)/*(...) repeat machinery, and some of those shapes nest an unbounded quantifier inside another one. Without further changes, +(?*a) would go from an inert literal on 4.0.5 to a backtracking bomb:

+(?*a)	compiled source	'a'.repeat(42) + '!'
4.0.5	^(?:\+(\?[^/]*?a))$	0 ms
detection fix alone	^(?:(?=.)(?:[^/][^/]*?a)+)$	1761 ms
this PR	^(?:\+\(\?\*a\))$	0 ms

analyzeRepeatedExtglob did not catch it: the /^[*?]+$/ risky-branch heuristic only runs when branches.length > 1, and ?*a is a single branch. So this PR also adds a hasMixedWildcards check that runs for every branch count:

if (branches.some(hasMixedWildcards)) {
  return { risky: true };
}

A branch that mixes a top-level ? with a top-level * (?*a, *?a) is treated as risky and the extglob is literalized, exactly as the existing guard already does for *(*|X). That closes +(?*a), *(?*a), +(?*[ab]) and +(?*a|b), and it is pinned by a new test in test/options.maxExtglobRecursion.js with both a source assertion and a timing bound.

Two honest notes on that:

• The check is symmetric, so it also literalizes the mirror-image shapes that are already exponential on master — +(*?a) takes 1093 ms on 4.0.5 at n=36 and is 0 ms here, and *(*?) inside a body is literalized too. That is a behaviour change to patterns unrelated to a leading ?. I judged the symmetry more defensible than a rule contorted to fire only on newly-reachable bodies, but if you would rather have that split into its own PR, say so and I will narrow it.
• It does not fix Security Audit: picomatch v4.0.4 | Incomplete ReDoS Fix Bypass (P1) #175 generally. +(*a) still compiles to ^(?:(?=.)(?:[^/]*?a)+)$ on both master and this branch, and still takes ~53 s on 'a'.repeat(30) + '!'. That is untouched here and out of scope.

Relationship to #191 — these two conflict

#191 ("metacharacters following a closing paren") and this PR both modify the same line in the ? handler:

- if (prev && prev.type === 'paren') {
+ if (prev && prev.type === 'paren' && !afterExtglob()) {          // #191
+ if (prev && prev.type === 'paren' && !afterExtglobOpenParen()) { // this PR

They are semantically complementary — #191 is about a metacharacter after the closing ), this is about a ? right after the opening ( — but they will conflict textually, so I named my helper afterExtglobOpenParen rather than something a character away from #191's afterExtglob. Whichever lands second, the resolution is to compose both conditions:

if (prev && prev.type === 'paren' && !afterExtglob() && !afterExtglobOpenParen()) {

I am happy to rebase on top of #191 if you take that one first.

One existing assertion changed

I am flagging this rather than burying it. test/extglobs.js:

it('should escape special characters immediately following opening parens', () => {
  assert(isMatch('cbz', 'c!(.)z'));    // bash: match      ✓
  assert(!isMatch('cbz', 'c!(*)z'));   // bash: no match   ✓
  assert(isMatch('cccz', 'c!(b*)z'));  // bash: match      ✓
  assert(isMatch('cbz', 'c!(+)z'));    // bash: match      ✓
  assert(isMatch('cbz', 'c!(?)z'));    // bash: NO match   ✗
  assert(isMatch('cbz', 'c!(@)z'));    // bash: match      ✓
});

Five of the six agree with bash. The ? one does not, because ? — unlike ., + and @ — is a glob metacharacter, exactly like the * on the second line that picomatch already handles correctly. [[ cbz == c!(?)z ]] is not a match in bash, and minimatch('cbz', 'c!(?)z') is false. I flipped that one assertion and left a comment.

Honest caveat: c!(?)z is still wrong afterwards, just differently. It now matches nothing, where bash matches cz and cbbz. That is the separate, pre-existing problem that a mid-pattern !() lookahead is not $-anchored (#93, #154). Net on that pattern is 0/3 correct → 1/3; I did not try to fix the anchoring.

Verification

npm test: 1982 passing, up from 1977 on master. npm run lint clean. npm run test:ci clean.

The three bug-covering assertions fail on a rebuilt master (git show master:lib/parse.js > lib/parse.js, confirmed the added helpers are absent, then re-run); the two guard tests (regex groups still groups, noextglob opt-out) pass both before and after.

I found this by differentially testing against bash 3.2 with shopt -s extglob, generating pattern/subject pairs from a small glob grammar (literals, *, ?, brackets, POSIX classes, nested extglobs) over the alphabet {a,b,c}. Subjects are slash-free so bash's [[ str == pat ]] semantics line up, and I excluded [!…]/[^…] patterns because that divergence is already #187. Note that shopt -s extglob only affects parsing of later commands, so the oracle evaluates each pair through eval in an already-extglob-enabled shell; I validated the batched oracle against per-pair bash -c runs before trusting it.

1085 patterns × 41 subjects = 44,485 pairs:

	disagreements with bash	distinct patterns
4.0.5	6050	289
this PR	4565	218

2122 pairs newly correct, 637 pairs newly wrong across 39 distinct patterns. Every one of the 39 falls into one of four classes, none of which is new logic in this PR:

class	pairs	patterns
the safety guard literalizes the extglob (#182 + the new mixed-wildcard rule above)	326	14
!(?…) negation, made visible by the unanchored mid-pattern lookahead (#93/#154)	142	12
a metacharacter after a closing ) binding as a regex quantifier (#191)	86	8
a nested extglob immediately inside an extglob body, still blocked by the isGroup check at lib/parse.js:1032	83	5

Concrete examples, one per class, in the form (subject, pattern) → bash / 4.0.5 / this PR:

guard      isMatch('a',   '*(?|a)')                   true  / true  / false
guard      isMatch('a',   '*(*(??|?)?|?[a])')         true  / true  / false
negation   isMatch('bab', '!(?(a)[ab])b')             true  / true  / false
closing )  isMatch('aa',  '*(?[[:digit:]])?(?)?')     true  / true  / false
nested     isMatch('aaa', '??(?([[:alnum:]]))')       false / false / true

Two more from the guard class that are worth seeing plainly: isMatch('a', '*(?|a)') and isMatch('aa', 'a*(?c|?)') were both true on 4.0.5 and are false here.

The first class is the guard doing its job: *(?|a) compiles to (?:[^/]|a)*, which is ambiguous and backtracks, so /^[*?]+$/ flags it and the extglob degrades to a literal. That is the same treatment *(*|a) already gets on master. It is still a real behaviour change for those patterns and I would rather you see the number than a rounded-down one.

What I could not verify

• Run on macOS / Node 26 only. I did not exercise the node 12–25 × ubuntu/windows/macos matrix. The change is pure tokenizer logic with no platform branches and is ES2018-safe, but I have not seen the matrix go green.
• The bash oracle is 3.2 (macOS system bash), which has extglob but not globstar, and [[ == ]] does no filename expansion. Globstar, dotfile and path-separator semantics are outside what I tested.
• '' still never matches *(?)/?(?), where bash does match. That is picomatch's blanket empty-input rule; unchanged and out of scope.
• I did not run micromatch/fast-glob/globby/chokidar suites against the patched build.
• The ReDoS accounting above is limited to the shapes I found. I did not do a systematic search for other single-branch bodies that nest unbounded quantifiers, and as noted +(*a) remains exponential on both master and this branch.

Rough microbenchmark of makeRe over 8 typical patterns × 20k iterations showed no regression.

[maximilliangrand]

Re-measured the ReDoS interaction on a clean checkout of this branch against a freshly installed picomatch@4.0.5, and the mirror-image case is considerably worse on master than the figure I quoted in the description. At 'a'.repeat(42) + '!':

pattern       4.0.5                                          this branch
+(?*a)        0.1 ms   ^(?:\+(\?[^/]*?a))$                   0.2 ms   ^(?:\+\(\?\*a\))$
*(?*a)        0.0 ms   ^(?:(?!\.)(?=.)[^/]*?(\?[^/]*?a))$    0.0 ms   ^(?:\*\(\?\*a\))$
+(?*[ab])     0.0 ms   ^(?:\+(\?[^/]*?(?:\[ab\]|[ab])))$     0.0 ms   ^(?:\+\(\?\*\[ab\]\))$
+(*?a)     18403.1 ms  ^(?:(?=.)(?:[^/]*?[^/]a)+)$           0.0 ms   ^(?:\+\(\*\?a\))$

So the hasMixedWildcards check does two things, and the second is the more valuable one:

1. it stops the new ?-leading bodies from reaching the repeat machinery unguarded (+(?*a) stays inert rather than becoming a bomb), and
2. it closes +(*?a), which is already exponential on released 4.0.5 — 18.4 seconds on a 43-character input, on a pattern that needs no ?-leading body at all.

That second one is a pre-existing instance of #175 that this branch happens to shut, and I had understated it at 1093 ms — that was measured at n=36, and the curve is steep enough that the exponent matters more than the constant.

I want to be equally clear about the cost, because it is the same symmetry: +(*?a) and friends previously matched (slowly) and now literalize, so they stop matching. That is a behaviour change to patterns unrelated to a leading ?, and it is on the list of newly-wrong pairs in the table above. If you would rather have the guard narrowed to only the newly-reachable bodies, or split out into its own PR ahead of this one, I am happy to do either — the split is clean, since the guard change and the parse change touch different functions.

Full suite on this branch: 1982 passing, 0 failing.