[rush] Use environment variables to pass publishing tokens

[octogonz]

Please prefix the issue title with the project name i.e. [rush], [api-extractor] etc.

Is this a feature or a bug?

• Feature
• Bug

Please describe the actual behavior.

Currently when publishing using Rush, the NPM token is passed via the command-line like this example:

- script: 'node common\scripts\install-run-rush.js version --bump --version-policy $(VersionPolicy) --target-branch $(Build.SourceBranchName)'
  displayName: 'Rush Version'
- script: 'node common\scripts\install-run-rush.js publish --apply --publish --include-all --target-branch $(Build.SourceBranchName) --npm-auth-token $(npmToken) --add-commit-details --set-access-level public'
  displayName: 'Rush Publish'

Although we use an environment variable $(npmToken), the variable gets expanded in the build log when invoking Rush:

node common\scripts\install-run-rush.js publish --apply --publish --include-all --target-branch master --npm-auth-token *** --add-commit-details --set-access-level public

...and when Rush shows how it was invoked:

Invoking "rush publish --apply --publish --include-all --target-branch master --npm-auth-token *** --add-commit-details --set-access-level public"

...and again when Rush executes PNPM:

* EXECUTING: D:\a\1\s\common\temp\pnpm-local\node_modules\.bin\pnpm publish --//registry.npmjs.org/:_authToken=*** --access public (libraries\package-deps-hash)

In all three cases, the authentication token gets replaced with *** because Azure DevOps is smart enough to detect the string and protect it. However, I've seen evidence that some other CI systems don't have this feature.

What is the expected behavior?

We should pass the tokens as environment variables instead of as command-line parameters. This would be much more secure, because environment variables don't get saved to disk, and aren't viewable by other processes running on the machine.

How to implement this

There are 3 pieces:

1. Update the Rush command-line to accept environment variables. The ts-command-line library makes this very easy; we would just specify the IBaseCommandLineDefinition.environmentVariable property for parameters like --npm-auth-token
2. When we invoke PNPM, pass the credentials as environment variables. If that's very difficult to do, another option would be to change the * EXECUTING: log message so that it deletes the token from the log output, rather than relying on Azure DevOps to do this.
3. Update the Rush website examples to illustrate using the environment variables, rather than using the command-line parameters. And mention that this is a best practice for security.

[darsi-an]

What's the best approach for debugging and testing a feature such as this? I currently debug rush using using VS Code debugger on the rush-example project. This worked for my previous PR, but not so much for this. I've also set up my own azure pipeline, but I get a non zero return status when verifying change logs.

[octogonz]

The VS Code debugger allows you to specify environment variables using the env setting;

{
  // Use IntelliSense to learn about possible attributes.
  // Hover to view descriptions of existing attributes.
  // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
  "version": "0.2.0",
  "configurations": [
    {
      "type": "node",
      "request": "launch",
      "name": "Launch Program",
      "program": "${workspaceFolder}\\lib\\start.js",
      "cwd": "C:\\path\\to\\rush-example",
      "env": {
        "EXAMPLE_VARIABLE": "example value"
      },
      "args": [ "version", "--bump", "--version-policy", "your-policy", "--target-branch", "master" ]
    }
  ]
}

I'm not sure you would need Azure DevOps to test this, except maybe for the last step. All the publishing operations can be run locally on a developer machine.

There's also some general tips here: https://github.com/microsoft/rushjs.io-website/issues/30