Skip to content

chore(deps): Bumped tmp and overrode uuid to address security advisories#8

Open
billythekid wants to merge 1 commit into
wxt-dev:masterfrom
billythekid:billythekid/security-tmp-uuid-overrides
Open

chore(deps): Bumped tmp and overrode uuid to address security advisories#8
billythekid wants to merge 1 commit into
wxt-dev:masterfrom
billythekid:billythekid/security-tmp-uuid-overrides

Conversation

@billythekid
Copy link
Copy Markdown

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). tmp was pinned exactly to 0.2.5, so it can't be picked up by npm update. Bumped to ^0.2.6, which resolves to 0.2.7.
  • uuid < 11.1.1: GHSA-w5hq-g745-h8pq. Pulled in transitively via node-notifier > uuid@8.3.2. Added a top-level overrides entry pinning uuid to ^11.1.1.

The override approach is needed because node-notifier looks abandoned. Its last release was 10.0.1 in February 2022, and an open PR to drop uuid in favour of crypto.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.js and scripts/develop.js), both of which are simple notifier.notify({ title, message, ... }) calls. The public showDesktopNotification API 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-notifier exists because cross-platform desktop notifications are awkward. On macOS it shells out to terminal-notifier (a bundled binary) or AppleScript. On Linux it uses notify-send (libnotify). On Windows it bundles SnoreToast.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 like BurntToast).

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-notifier entirely can follow as a separate change if you think it's worth it.

One alternative worth flagging: toasted-notifier is a fork of node-notifier 10.x with the same API (drop-in), no uuid or tmp in 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 swapping node-notifier for 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.0 to 8.3.0, which is the version that introduced the overrides field. Node 18+ ships with npm well above 8.3.0, so every supported environment already meets this.

Verification

  • npm install resolves cleanly.
  • npm run build succeeds.
  • npm test reports 360 passing / 18 failing, the same counts as master. The 18 failures live in tests/unit/test-cmd/test.run.js and are unrelated to these changes (they reproduce on a clean checkout of master).
  • tmp and uuid no longer appear in npm audit output.
  • Lockfile confirms tmp@0.2.7 and uuid@11.1.1 as the resolved versions.

- 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).
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.

1 participant