chore(deps): Bumped tmp and overrode uuid to address security advisories#8
Open
billythekid wants to merge 1 commit into
Open
Conversation
- Bumped tmp from 0.2.5 to ^0.2.6 to address GHSA-ph9p-34f9-6g65. - Added an overrides entry pinning uuid to ^11.1.1 to address GHSA-w5hq-g745-h8pq, which affects uuid <11.1.1 reachable through the node-notifier dependency. - Bumped the minimum npm engine to 8.3.0 (required for the overrides field).
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
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.
First-time contribution to this repo, happy to adjust scope, structure, or commit style if anything doesn't match how you'd prefer.
Summary
Two transitive security advisories currently surface for downstream users (via
npm audit/ GitHub Dependabot):tmp< 0.2.6: GHSA-ph9p-34f9-6g65 (arbitrary file/directory write via symlink).tmpwas pinned exactly to0.2.5, so it can't be picked up bynpm update. Bumped to^0.2.6, which resolves to0.2.7.uuid< 11.1.1: GHSA-w5hq-g745-h8pq. Pulled in transitively vianode-notifier > uuid@8.3.2. Added a top-leveloverridesentry pinninguuidto^11.1.1.The override approach is needed because
node-notifierlooks abandoned. Its last release was10.0.1in February 2022, and an open PR to dropuuidin favour ofcrypto.randomUUID()has sat without maintainer response since October 2022. With no upstream release likely, an override is the only practical fix for downstream consumers.Why not just drop
node-notifier?(This bit I got AI to check for me)
It's used in two places (
src/util/desktop-notifier.jsandscripts/develop.js), both of which are simplenotifier.notify({ title, message, ... })calls. The publicshowDesktopNotificationAPI could be kept identical, so no callers would need to change.The reason not to do it in this PR is the implementation side.
node-notifierexists because cross-platform desktop notifications are awkward. On macOS it shells out toterminal-notifier(a bundled binary) or AppleScript. On Linux it usesnotify-send(libnotify). On Windows it bundlesSnoreToast.exe/toaster.exe. Replacing it properly means either bundling those binaries yourselves or accepting a regression on at least one platform (most likely Windows, where there's no built-in equivalent without something likeBurntToast).That's a design decision for yourself. Keeping this one as a small dependency bump means it can land quickly and unblock downstream consumers who are seeing the advisories today. Dropping
node-notifierentirely can follow as a separate change if you think it's worth it.One alternative worth flagging:
toasted-notifieris a fork ofnode-notifier10.x with the same API (drop-in), nouuidortmpin its dependency tree, and a release ~11 months ago. It's a single-maintainer project with only two published versions so far, so the supply-chain trade-off is different rather than strictly better, but swappingnode-notifierfor it would resolve both advisories without needing an override. Given the fork relationship, you might also want to reach out to the maintainer directly.npm engine bump
The minimum npm engine moves from
8.0.0to8.3.0, which is the version that introduced theoverridesfield. Node 18+ ships with npm well above 8.3.0, so every supported environment already meets this.Verification
npm installresolves cleanly.npm run buildsucceeds.npm testreports 360 passing / 18 failing, the same counts asmaster. The 18 failures live intests/unit/test-cmd/test.run.jsand are unrelated to these changes (they reproduce on a clean checkout ofmaster).tmpanduuidno longer appear innpm auditoutput.tmp@0.2.7anduuid@11.1.1as the resolved versions.