tools: delete macos installer postinstall script #57661

sdavids · 2025-03-28T11:42:04Z

Refs: https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html#//apple_ref/doc/uid/TP40004268-CH8-SW2
Fixes: #57660

sdavids · 2025-03-28T11:56:01Z

Here is how one can modify the PATH used by GUI apps—in this case the installer:

$ sudo launchctl config user --help
Usage: launchctl config <system|user> <parameter> <value>
When given the "system" argument, modifies the configuration for the system
domain. When given the "user" argument on supported platforms, modifies the
configuration for all user domains. You must reboot for changes to take effect.
Note that if a service specifies a conflicting configuration, the service's
parameter will be preferred.

Supported configuration parameters are:

umask <integer as octal>
Modifies the umask(2) applied to services launched in the domain.

path <string>
Modifies the PATH environment variable set on each service in the
domain.

aduh95 · 2025-03-28T13:45:36Z

Is this file still used? According to #4635 (comment), it's not run by the installer, and the only reference to it I could fine is https://github.com/nodejs/node-convergence-archive/blob/e11fe0c2777561827cdb7207d46b0917ef3c42a7/tools/osx-pkg.pmdoc/02npm.xml 🤔

sdavids · 2025-03-28T17:18:50Z

Yes, see
#57548 (comment)

ljharb · 2025-03-28T17:20:52Z

What if the binaries aren't available on that PATH? (they might be brew-installed, for example) It seems correct to me to use whatever is on the PATH.

It's not an attack if it requires you to attack yourself.

sdavids · 2025-03-28T18:05:27Z

System Integrity Protection on your Mac

System Integrity Protection includes protection for these parts of the system:
[…]
/bin
[…]
Paths and apps that third-party apps and installers can continue to write to include:
[…]
/usr/local
[…]
System Integrity Protection is designed to allow modification of these protected parts only by processes that are signed by Apple and have special entitlements to write to system files, such as Apple software updates and Apple installers.

The NodeJS installer extracts to /usr/local/bin and creates symlinks in the same directory—this is fine with SIP.

This is about /bin/ln.

homebrew

https://docs.brew.sh/FAQ#why-should-i-install-homebrew-in-the-default-location

The default prefix is:
[…]
/opt/homebrew for macOS on Apple Silicon,
/usr/local for macOS on Intel, and
[…]
Do yourself a favour and install to the default prefix so that you can use our pre-built binary packages. Pick another prefix at your peril!

https://formulae.brew.sh/formula/coreutils

Commands also provided by macOS and the commands dir, dircolors, vdir have been installed with the prefix "g".
If you need to use these commands with their normal names, you can add a "gnubin" directory to your PATH with:
PATH="$HOMEBREW_PREFIX/opt/coreutils/libexec/gnubin:$PATH"

Note: This only works for commands started via a terminal app. If you wanted them to be accessible to user apps (like the NodeJS installer) you would need to do (on an ARM machine):

$ sudo launchctl config user path '/opt/homebrew/opt/coreutils/libexec/gnubin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin'
$ sudo reboot

MacPorts

https://guide.macports.org/chunked/internals.configuration-files.html

prefix
Sets the directory where ports are installed.
[…]
Default: /opt/local

https://trac.macports.org/wiki/FAQ#defaultprefix

Traditionally […] /usr/local. However, having MacPorts under /usr/local would be error-prone for precisely that reason. Many other software packages and packaging systems install into /usr/local, and could accidentally overwrite what MacPorts has installed, or vice versa […] MacPorts doesn't want to be a victim of that, and /opt/local provides the splendid isolation […]

TL;DR

When SIP is enabled (default in all modern macOS versions) then third-party installers, i.e. the NodeJS installer, and superusers cannot modify the contents of /bin.

That is why this hardening is useful.

A malicious entity cannot prepend the user/system path on a user’s host via sudo launchctl config [user|systen] path and then wait for someone to run the NodeJS installer.

The malicious ln would then run with administrator privileges and 💥

ljharb · 2025-03-28T18:07:28Z

so the attack vector is that i can inject an unprivileged executable, and by way of another script, be sudo'd into a privileged context? 🤔

aduh95 · 2025-03-28T18:15:46Z

Is this file still used?

Yes, see #57548 (comment)

This seems to indicate it is not used, AFAICT it only mentions

node/tools/macos-installer/pkgbuild/npm/scripts/postinstall

Lines 1 to 5 in 03954f7

    
           #!/bin/sh 
        
           cd /usr/local/bin || exit 1 
        
           ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm 
        
           ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx

sdavids · 2025-03-28T18:24:52Z

macOS is a multi-user system, even if most people do not use it that way.

The attack vector is:

A)

prepending the user path with a directory (e.g. homebrew or MacPorts bin dir); this could have been done by your company's admins for various reasons
placing a malicious ln into that directory

B)

place a malicious ln into /usr/local/bin

A different user just needs to execute the NodeJS installer.

Quoting Apple here:

https://developer.apple.com/library/archive/documentation/OpenSource/Conceptual/ShellScripting/ShellScriptSecurity/ShellScriptSecurity.html#//apple_ref/doc/uid/TP40004268-CH8-SW2

Always specify absolute or relative paths when executing binaries or other scripts.

sdavids · 2025-03-28T18:34:51Z

This seems to indicate it is not used, AFAICT it only mentions

The scripts are executed when you execute the installer, not when you use pkgutil --expand.

I just found out that there is another postinstall script in the repo:

https://github.com/nodejs/node/blob/main/tools/macos-installer/pkgbuild/npm/scripts/postinstall

Maybe one of them should be deleted to lessen the confusion?

aduh95 · 2025-03-28T18:41:41Z

I've opened #57667 to remove it

sdavids · 2025-03-28T19:09:53Z

I found where the scripts are used:

https://github.com/npm/node/blob/2bf0bd2c9d25b810076eec6b2fdaf333f4a1eac2/Makefile#L179-L1183

	pkgbuild --version $(NPMVERSION) \
		--identifier org.nodejs.npm.pkg \
		--root $(MACOSOUTDIR)/dist/npm \
		--scripts ./tools/macos-installer/pkgbuild/npm/scripts \
			$(MACOSOUTDIR)/pkgs/npm-$(NPMVERSION).pkg

$ man pkgbuild
NAME
     pkgbuild – Build a macOS Installer component package from on-disk files
...
     --scripts scripts-path
             […] If this directory contains scripts named preinstall and/or postinstall, these will be ran as the top-level scripts of the package […]

sdavids · 2025-03-30T20:57:21Z

I have updated the PR.

I moved the symlink creation into the Makefile thereby making the postinstall script obsolete.

This will fix #57660 and #57548.

This is in addition to #57667.

$ tree --noreport out/macos/dist/npm/usr/local/bin
out/macos/dist/npm/usr/local/bin
├── npm -> ../lib/node_modules/npm/bin/npm-cli.js
└── npx -> ../lib/node_modules/npm/bin/npx-cli.js
$ tree --noreport out/macos/pkgs 
out/macos/pkgs
├── node-v24.0.0.pkg
└── npm-v11.2.0.pkg
$ pkgutil --expand out/macos/pkgs/node-v24.0.0.pkg /tmp/node 
$ pkgutil --expand out/macos/pkgs/npm-v11.2.0.pkg /tmp/npm
$ tar -tvf /tmp/node/Payload | grep './usr/local/bin'        
drwx------  4 0      0           0 Mar 30 22:15 ./usr/local/bin
-rwx------  1 0      0   231691704 Mar 30 22:15 ./usr/local/bin/node
lrwx------  1 0      0          45 Mar 30 22:14 ./usr/local/bin/corepack -> ../lib/node_modules/corepack/dist/corepack.js
$ tar -tvf /tmp/npm/Payload | grep './usr/local/bin' drwx------  4 0      0           0 Mar 30 22:39 ./usr/local/bin
lrwx------  1 0      0          38 Mar 30 22:39 ./usr/local/bin/npx -> ../lib/node_modules/npm/bin/npx-cli.js
lrwx------  1 0      0          38 Mar 30 22:39 ./usr/local/bin/npm -> ../lib/node_modules/npm/bin/npm-cli.js

Note: The lrwx------ is due to my machine having umask 022; on a regular build machine it will be lrwxr-xr-x.

Fixes: nodejs#57660 Fixes: nodejs#57548. Signed-off-by: Sebastian Davids <[email protected]>

aduh95 · 2025-03-30T21:09:08Z

Makefile

+	mkdir -p $(MACOSOUTDIR)/dist/npm/usr/local/bin
+	ln -sf ../lib/node_modules/npm/bin/npm-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npm
+	ln -sf ../lib/node_modules/npm/bin/npx-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npx


That doesn't seem correct, we don't want to install those on the release machine

Suggested change

mkdir -p $(MACOSOUTDIR)/dist/npm/usr/local/bin

ln -sf ../lib/node_modules/npm/bin/npm-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npm

ln -sf ../lib/node_modules/npm/bin/npx-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npx

You can either place these files on the release machine before invoking pkgbuild and then they are inside the package and no postinstall is needed or you need the postinstall script.

One could remove them right after the pkgbuild though.

I am also unsure what you mean by "release machine".

These symlinks are created inside the build folder out which is gitignored.

$ tree --noreport out/macos/dist/npm/usr/local/bin out/macos/dist/npm/usr/local/bin ├── npm -> ../lib/node_modules/npm/bin/npm-cli.js └── npx -> ../lib/node_modules/npm/bin/npx-cli.js $ tree --noreport out/macos/pkgs out/macos/pkgs ├── node-v24.0.0.pkg └── npm-v11.2.0.pkg $ pkgutil --expand out/macos/pkgs/node-v24.0.0.pkg /tmp/node $ pkgutil --expand out/macos/pkgs/npm-v11.2.0.pkg /tmp/npm $ tar -tvf /tmp/node/Payload | grep './usr/local/bin' drwx------ 4 0 0 0 Mar 30 22:15 ./usr/local/bin -rwx------ 1 0 0 231691704 Mar 30 22:15 ./usr/local/bin/node lrwx------ 1 0 0 45 Mar 30 22:14 ./usr/local/bin/corepack -> ../lib/node_modules/corepack/dist/corepack.js $ tar -tvf /tmp/npm/Payload | grep './usr/local/bin' drwx------ 4 0 0 0 Mar 30 22:39 ./usr/local/bin lrwx------ 1 0 0 38 Mar 30 22:39 ./usr/local/bin/npx -> ../lib/node_modules/npm/bin/npx-cli.js lrwx------ 1 0 0 38 Mar 30 22:39 ./usr/local/bin/npm -> ../lib/node_modules/npm/bin/npm-cli.js

Note: The lrwx------ is due to my machine having umask 022; on a regular build machine it will be lrwxr-xr-x.

Moving the unlink call to after invocating pkgbuild sounds like a better solution

unlink $(MACOSOUTDIR)/dist/node/usr/local/bin/npx

The unlinks are for the npm and npx symlinks in the node and not the npm directory; these would have been placed in the org.nodejs.node.pkg package—the reason being that they won't end up in the package and will not get extracted to /usr/local/bin on the machine the installer is executed on.

The symlinks created by this PR will be placed in the org.nodejs.npm.pkg package and thereby the installer will extract them into /user/local/bin with the correct permissions and user/group.

One could add 2 additional unlink statements for npm and npx after the pkgbuild though.

But I am not sure if it is worth it: Aren't these links in an ephemeral directory on a build machine?

nodejs-github-bot added macos Issues and PRs related to the macOS platform / OSX. tools Issues and PRs related to the tools directory. labels Mar 28, 2025

sdavids changed the title ~~tools: harden scripts~~ tools: harden macOS postinstall script Mar 28, 2025

sdavids force-pushed the users/sdavids/fix/57660 branch from b6187d0 to fdaa5c2 Compare March 28, 2025 18:50

sdavids force-pushed the users/sdavids/fix/57660 branch from fdaa5c2 to 7d82303 Compare March 30, 2025 20:52

tools: delete macos installer postinstall script

1426339

Fixes: nodejs#57660 Fixes: nodejs#57548. Signed-off-by: Sebastian Davids <[email protected]>

sdavids force-pushed the users/sdavids/fix/57660 branch from 7d82303 to 1426339 Compare March 30, 2025 21:00

This was referenced Mar 30, 2025

tools: explicitly set umask in macOS postinstall script #57662

Closed

Mac Installer: npm and npx created with wrong permissions #57548

Open

aduh95 reviewed Mar 30, 2025

View reviewed changes

sdavids changed the title ~~tools: harden macOS postinstall script~~ tools: delete macos installer postinstall script Mar 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: delete macos installer postinstall script #57661

tools: delete macos installer postinstall script #57661

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

aduh95 commented Mar 28, 2025

sdavids commented Mar 28, 2025

ljharb commented Mar 28, 2025

sdavids commented Mar 28, 2025 •

edited

Loading

ljharb commented Mar 28, 2025

aduh95 commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

aduh95 commented Mar 28, 2025

sdavids commented Mar 28, 2025

sdavids commented Mar 30, 2025 •

edited

Loading

aduh95 Mar 30, 2025

sdavids Mar 30, 2025 •

edited

Loading

sdavids Mar 30, 2025 •

edited

Loading

aduh95 Mar 30, 2025

sdavids Mar 30, 2025 •

edited

Loading

	mkdir -p $(MACOSOUTDIR)/dist/npm/usr/local/bin
	ln -sf ../lib/node_modules/npm/bin/npm-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npm
	ln -sf ../lib/node_modules/npm/bin/npx-cli.js $(MACOSOUTDIR)/dist/npm/usr/local/bin/npx

tools: delete macos installer postinstall script #57661

Are you sure you want to change the base?

tools: delete macos installer postinstall script #57661

Conversation

sdavids commented Mar 28, 2025 • edited Loading

sdavids commented Mar 28, 2025 • edited Loading

aduh95 commented Mar 28, 2025

sdavids commented Mar 28, 2025

ljharb commented Mar 28, 2025

sdavids commented Mar 28, 2025 • edited Loading

homebrew

MacPorts

TL;DR

ljharb commented Mar 28, 2025

aduh95 commented Mar 28, 2025 • edited Loading

sdavids commented Mar 28, 2025 • edited Loading

sdavids commented Mar 28, 2025 • edited Loading

aduh95 commented Mar 28, 2025

sdavids commented Mar 28, 2025

sdavids commented Mar 30, 2025 • edited Loading

aduh95 Mar 30, 2025

Choose a reason for hiding this comment

sdavids Mar 30, 2025 • edited Loading

Choose a reason for hiding this comment

sdavids Mar 30, 2025 • edited Loading

Choose a reason for hiding this comment

aduh95 Mar 30, 2025

Choose a reason for hiding this comment

sdavids Mar 30, 2025 • edited Loading

Choose a reason for hiding this comment

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

aduh95 commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 28, 2025 •

edited

Loading

sdavids commented Mar 30, 2025 •

edited

Loading

sdavids Mar 30, 2025 •

edited

Loading

sdavids Mar 30, 2025 •

edited

Loading

sdavids Mar 30, 2025 •

edited

Loading