doc: add section about using npx with permission model

[RafaelGSS]

cc: @wraithgar

[nodejs-github-bot]

Review requested:

• @nodejs/security-wg

[wraithgar]

@RafaelGSS and I tested this locally w/ both npx and npm exec. This is the preferred way to set these flags, as it works in virtually all existing versions of npm already and does exactly what we need. Future changes to these flags only requires updating the flags you set via this parameter, and no changes to npm would be needed.

[AugustinMauroy]

maybe add link to npx docs for people not 100% aware of what is npx

https://docs.npmjs.com/cli/v8/commands/npx

[wraithgar]

If you link please link to the latest version, not npm 8

https://docs.npmjs.com/cli/commands/npx

[wraithgar]

Realized that this won't work for packages installed globally and for packages installed in the npx cache. The example given is for if the package is installed in the current package at cwd.

Not sure if there's a good way to document what --allow-fs-read needs to be in other cases, and why. But here's what will work:

Examples using fish shell

# installed globally
$ npm ls -g
/Users/wraithgar/.nvm/versions/node/v22.13.0/lib
├── [email protected]
$ npx --node-options="--permission --allow-fs-read=$(npm prefix -g)" json --version
json 11.0.0
written by Trent Mick
https://github.com/trentm/json

# using npx cache
$ npx --node-options="--permission --allow-fs-read=$(npm config get cache)" semver parse 1.0.0
1.0.0

This seems like a lot to try to document for this flag.

[marco-ippolito]

Maybe we can wrap the permission model as an npx flag? As a new npx feature

[wraithgar]

I would discourage adding a new npm config for this, as it would mean it wouldn't work for existing versions of npx, and in a way that it was silently ignored so you wouldn't know it wasn't working.

Also it would mean having to keep npm in sync w/ these params if they ever changed or were added to. It would be completely decoupled from Node.js config itself, meaning someone would just have to "remember" to do it if Node.js changed.

Showing users how to set the flags in npx is the best option, as it allows them to update the flags w/o having to wait on npm to update, and already works.

[marco-ippolito]

I would discourage adding a new npm config for this, as it would mean it wouldn't work for existing versions of npx, and in a way that it was silently ignored so you wouldn't know it wasn't working.

Also it would mean having to keep npm in sync w/ these params if they ever changed or were added to. It would be completely decoupled from Node.js config itself, meaning someone would just have to "remember" to do it if Node.js changed.

Showing users how to set the flags in npx is the best option, as it allows them to update the flags w/o having to wait on npm to update, and already works.

If you add a command line flag to npx, in a version that does not support it, it should should throw an unrecognized flag, I'd not expect npx --foobar to fail silently (on the phone cant test but I expect it fails).
Unit tests that can keep "in sync" the permission model flags and npm if are removed from Node.
But anyways my suggestion applies only if npx cannot properly support passing permission model flags.
I also found some struggle resolving paths through shebang (I couldnt make it work properly) so if not possible or not ergonomic, we should find a better solution.

[wraithgar]

If you add a command line flag to npx, in a version that does not support it, it should should throw an unrecognized flag,

We are working towards having npm do this, unfortunately because it doesn't do that now nobody using npx today would get an error. npm 12 (and npx by extension) should throw on unknown config.

[RafaelGSS]

To be honest, I think documenting it is the best approach we could do to users, they will be 100% aware of paths they are giving access to, and it does make total sense for me having to give access explicitly to npx cache folder for instance.

In the case of shebang, it goes a bit out of permission model scope, this is more unix path resolution, I don't think we should do something to make it "easier". This is a standard pattern.

[marco-ippolito]

If you add a command line flag to npx, in a version that does not support it, it should should throw an unrecognized flag,

We are working towards having npm do this, unfortunately because it doesn't do that now nobody using npx today would get an error. npm 12 (and npx by extension) should throw on unknown config.

I see then it makes totally sense! Thanks

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/56539
✔  Done loading data for nodejs/node/pull/56539
----------------------------------- PR info ------------------------------------
Title      doc: add section about using npx with permission model (#56539)
Author     Rafael Gonzaga <[email protected]> (@RafaelGSS)
Branch     RafaelGSS:mention-npx-node-options -> nodejs:main
Labels     doc, author ready
Commits    1
 - doc: add section about using npx with permission model
Committers 1
 - RafaelGSS <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/56539
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/56539
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - doc: add section about using npx with permission model
   ℹ  This PR was created on Thu, 09 Jan 2025 21:49:15 GMT
   ✔  Approvals: 2
   ✔  - Trivikram Kamat (@trivikr): https://github.com/nodejs/node/pull/56539#pullrequestreview-2541035334
   ✔  - Ruy Adorno (@ruyadorno) (TSC): https://github.com/nodejs/node/pull/56539#pullrequestreview-2541159264
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/12727899336

[nodejs-github-bot]

Landed in b22c3d3