Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: update glob-parent@6 #331

Closed
wants to merge 1 commit into from
Closed

refactor: update glob-parent@6 #331

wants to merge 1 commit into from

Conversation

zorji
Copy link

@zorji zorji commented Sep 18, 2021

What is the purpose of this pull request?

Update glob-parent to fix a reported vulnerability

This is a major upgrade, it no longer supports Node v10 and there are other breaking change

I can see that there are no tests cases included in this package.

What changes did you make? (Give an overview)

  • upgrade glob-parent to the next major version that fixed a reported vulnerability

@mrmlnc
Copy link
Owner

mrmlnc commented Oct 31, 2021

Thanks for the contribution, but the problem has already been fixed in one of the latest releases:

https://github.com/mrmlnc/fast-glob/releases/tag/3.2.6

@mrmlnc mrmlnc closed this Oct 31, 2021
@lux01
Copy link

lux01 commented Dec 14, 2021

@mrmlnc Sorry the changes made in 3.2.6 are insufficient to fix CVE-2021-35065, glob-parent@6.0.1 must be used to fix the issue as per https://security.snyk.io/vuln/SNYK-JS-GLOBPARENT-1314294 but the changes made in b7458b8 only move fast-glob to glob-parent@5.1.2.

@mrmlnc mrmlnc added this to the 4.0.0 milestone Jan 4, 2022
@mrmlnc
Copy link
Owner

mrmlnc commented Jan 4, 2022

You're right. Sorry.

Unfortunately, the new version of the glob-parent package requires node.js 10+. This package supports node.js 8+. My position regarding this security issue.

Also an important note: gulpjs/glob-parent#49 (comment) and gulpjs/glob-parent#49 (comment).

Because of this, I am move this fix to the next major version.

@paulmillr
Copy link

@lux01 you are wrong. Glob-parent 5.1.2 is not vulnerable and can be safely used. Use the google and search button.

@mrmlnc you don't need to drop support for node 8. Proceed to safely use glob-parent 5.1.2.

@mrmlnc
Copy link
Owner

mrmlnc commented Jan 10, 2022

Thanks @paulmillr for clarification. Yeap, I know about situation with CVE issues and other "very useful security services" (sarcasm).

Right now we use the glob-parent package with 5.1.2 version. In the next major version of this package we will use 6.0.2. Because of this, I am move this PR to the next major version in which I'm going to drop support for older versions.

@duailibe
Copy link

@mrmlnc FYI another reason to bump glob-parent is because of this bugfix: gulpjs/glob-parent#34.

What happens is if you have a file with braces in it, the task's base points to a path that's not a directory:

Steps to reproduce:

Create a file src/{foo} and then call fg.generateTasks(fg.escapePath('src/{foo}')), which returns:

[
  {
    dynamic: true,
    positive: [ 'src/\\{foo\\}' ],
    negative: [],
    base: 'src/{foo}',  // <-- not a folder
    patterns: [ 'src/\\{foo\\}' ]
  }
]

Because of this running fg.sync(fg.escapePath('src/{foo}')) fails with ENOTDIR (see prettier/prettier#12143 for the original bug report).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants