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

fix: remove unessessary executable bits #242

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

donatj
Copy link
Contributor

@donatj donatj commented Nov 30, 2023

I was doing a security scan which included our dependencies and I found that your LICENSE had it's executable bit set in our node_modules folder.

They almost certainly shouldn't be. I did a quick

$ find . -type f -exec test -x {} \; -print | grep -v '/\.git/'
./LICENSE
./.gitattributes

I then removed the executable bit from all of these as they are not scripts.

Let me know if you have any questions or comments.

@jonschlinkert
Copy link
Member

jonschlinkert commented Nov 30, 2023

It seems more likely that the package manager somehow did that. My hunch is that package manager is grouping license files together with files, directories, binaries, etc. defined in package.json. and when they install those files they all get the same directory permissions.

@donatj
Copy link
Contributor Author

donatj commented Nov 30, 2023

If you look at the diff here, they are executable in the repo currently, and they shouldn't be. I wouldn't have been able to open the PR with no diff.

@donatj
Copy link
Contributor Author

donatj commented Nov 30, 2023

image

The diff showing the permission change

@jonschlinkert
Copy link
Member

If you look at the diff here, they are executable in the repo currently, and they shouldn't be. I wouldn't have been able to open the PR with no diff.

This is irrelevant to my point.

@donatj
Copy link
Contributor Author

donatj commented Dec 1, 2023

The executable bit is currently set in the repository on the LICENSE file. Agreed? Can we also agree that it shouldn't be and it should be removed regardless?

The executable bit is set in the published package. I think that's your area of contention, but it is.

Here's a screenshot.

I've tried it on macOS and Linux.

You can try it for yourself, presuming you have access to a non-Windows OS that supports the executable bit.

$ npm pack micromatch
…
$ tar -ztvf micromatch-4.0.5.tgz
…

I don't see any reason to believe that the executable bit in the published package is set for reasons other than the original file being executable.

There would have to be a step that removed it, and then a second step that added it back. Occam's Razor says it's probably just copying the original permissions of the file, given they match.

@jonschlinkert jonschlinkert merged commit 2f4a045 into micromatch:master Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants