Don't auto-install lefthook if not found #602
Conversation
|
Relates to discussion: #599 |
internal/templates/hook.tmpl
Outdated
| then | ||
| npx @evilmartians/lefthook "$@" | ||
| npx --no-install @evilmartians/lefthook "$@" |
There was a problem hiding this comment.
I understand your concern. The @evilmartians/lefthook package indeed installs .exe file. I think it will be better to change this line to npx lefthook "$@".
If we add --no-install here it will confuse people because it will just print
npm ERR! canceled
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/user/.npm/_logs/2024-01-09T09_02_30_257Z-debug-0.log
It's a breaking change and it has to be thought out. Initial intention was to do a seamless run via npx.
There was a problem hiding this comment.
"It's a breaking change and has to be thought out" - totally understand, thanks for taking the time to look at this!
I'd rather just remove the npx lines in that case - for me the auto-install behaviour is problematic even without the .exe 😅 .
e.g. I run brew remove lefthook but then lefthook mysteriously re-installs itself in my npx cache
There was a problem hiding this comment.
Ok, I got it 👍 So this PR will wait until I decide to release a major update. Thank you for your contribution!
There was a problem hiding this comment.
Sounds good! Thanks for solving the .exe issue in the meantime. 😁
Closes # (issue)
#599
⚡ Summary
As-is, if lefthook is not found, it will auto-install the latest lefthook from npm. (npx auto-fetches by default)
This isn't great:
Security: npm isn't known for being the most secure platform, and if the package was the target of an attack, all lefthook users with node installed would be affected, even those who don't use the lefthook npm package.
User friendliness: installing binaries on the user's computer without their permission isn't great, and one of those lefthook binaries (an .exe file) was flagged by an anti-malware package at my organization.
It seems that the intention was to just print a message when lefthook is not found (which I agree with) and it's possible that using npx's fetch-if-not-installed behaviour was just an oversight when implementing this. That said, anyone who has node installed will not get that far, and will be subject to the auto-install behaviour.
☑️ Checklist