fix: Fixed files not being ignored when sourceDir contains glob characters - #3835
Open
MaxFreedomPollard wants to merge 1 commit into
Open
MaxFreedomPollard wants to merge 1 commit into
MaxFreedomPollard wants to merge 1 commit into
Conversation
…cters
Every ignore pattern is made absolute by prefixing the resolved sourceDir,
and that prefix was then read back as glob syntax. In a source directory
named "my ext [beta]" the "[beta]" became a one-character class, and an
extglob opener such as "@(", "+(", "*(" or "?(" in the name was parsed as a
pattern group, so no base pattern matched any file and .git, node_modules,
dotfiles and old zip files were all packed into the artifact. A negated
"!(" form did not fail open but matched sibling directories instead. Bare
parentheses were already treated literally and were never affected.
The sourceDir prefix is now escaped as one-character classes, which survives
the path separator rewrite minimatch does on Windows, and the artifacts
directory is escaped whole because it is a literal path and never a pattern.
A brace expansion such as "{a,b}" in a directory name stays unfixed, because
braces are expanded before character classes are parsed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If the directory an extension lives in has a glob character in its name, web-ext ignores nothing. A build from a folder like "/src/my ext [beta]" packs .git, node_modules, every dotfile and every previously built zip straight into the artifact, and any pattern the user put in ignoreFiles is silently dead too.
Every ignore pattern is made absolute by prefixing the resolved sourceDir, and minimatch then reads that prefix back as pattern syntax instead of as a real path. "[beta]" is parsed as a one-character class that matches one of b, e, t or a, so the pattern no longer matches the directory it came from and nothing is ignored. The extglob openers "@(", "+(", "*(" and "?(" fail the same way, and a "!(" name matches the wrong sibling directories. Bare parentheses were never a problem, since minimatch already treats a "(" with no extglob character in front of it as a literal.
The fix escapes the glob characters in the sourceDir prefix before the rest of the pattern is appended, so the directory is matched literally while the user's own pattern keeps its glob meaning. Each character is escaped as a one-character class, "[" becoming "[[]", rather than with a backslash, because on Windows minimatch rewrites the path separator to "/" inside patterns and a backslash escape would not survive that. The artifacts directory is a real path and never a pattern, so it is escaped whole.
Two cases stay unfixed. A curly brace in a directory name still breaks matching, because brace expansion runs before character classes are parsed and there is no class that can hide a brace from it. A literal backslash in a directory name on POSIX is also still read as an escape.
Two unit tests cover this: one builds a filter on "/src/my ext [beta]" and on "/src/build @(v2)" and checks that the base patterns and a user ignoreFiles entry with a negation all still behave, and one checks an artifacts directory whose own name contains a bracket class. Both fail on master. eslint and prettier --check pass on the two changed files.