rfc: Command to add a script to package.json

[TheJaredWilcurt]

Thing I wish existed. No opinion on the implementation or any of the bikes.

[isaacs]

In the bikeshedding: I think this should be npm add-script <name> <cmd>, and overwrite with a warning if one already exists. Otherwise, +1.

[ljharb]

Can it somehow be prevented from being invoked from postinstall scripts? otherwise it seems like a new attack vector.

[isaacs]

@ljharb That would just add a new script to the package.json of the package being installed? Ie, you could have something like this:

{
  "scripts": {
    "install": "npm add-script postinstall \"something evil\""
  }
}

But that strikes me as no more dangerous than:

  "scripts": {
    "postinstall": "something evil"
  }
}

That is, it's not safe per se, but I'm not sure if I see how it's a new attack vector.

[ljharb]

I guess it feels like anything you can do with npm is more privileged/trusted than anything you can do without - and adding a new script to the package doing the installing is what I’m concerned about.

[TheJaredWilcurt]

@isaacs @ljharb

Update PR based on feedback. All bikes addressed.

[isaacs]

More potential bikeshed ideas, which I am very much not pushing for, but want to make sure get logged here.

What if we extend npm run to be able to run arbitrary commands under an arbitrary script name, and then have it respect --save.

npm run start  # runs the scripts.start
npm run start "node server.js"  # runs node server.js, with env.npm_lifecycle_script="start"
npm run start "node server.js" --save  # same as above, but save to script.start

The hazard here is that npm run start <arg> currently runs the thing configured in package.json with the arg attached to the argument list. So it could be a bit of a footgun, and adding a new command might be better:

npm exec node server.js
npm exec --save-script=start node server.js

This would also bring some of the functionality of npx into the fold as a top-level npm command.

The safest and easiest approach is to add an add-script command, as this RFC proposes. The only thing that's a bit strange is that it uses "add" to "edit" an existing script, but I don't think that's too horrible.

[TheJaredWilcurt]

@isaacs I thought the name --set-script may be better than --add-script as it works for creating and replacing. I'm fine with either though

[isaacs]

Just one npm CLI pedantic note: in npm --xyz is a config, not a command. For example, in npm install foo --save, the --save is a modifier. You could also do npm install foo --no-save or npm config set save=false.

In this case, "set the script" is the action, so it should be a positional argument, not a config flag.

But yes, npm set-script start "node server.js" is better. I'll update the RFC and make a link to this comment in the commit. Good call.

[TheJaredWilcurt]

The wording for the warning message could be documented (or bike shed). My proposal was:

Warning: Existing "start" script has been overwritten. Replaced "express" with "nw .".