feat: support the npmrc tokenHelper auth setting - #2948
Conversation
|
|
Run a registry's tokenHelper executable and use its stdout as the bearer token, the same path a static _authToken takes. It is only honored from the trusted user-level auth file (pnpm's npmrcAuthFile, default ~/.npmrc, overridable with the npmrc_auth_file attribute or PNPM_CONFIG_NPMRC_AUTH_FILE), never a project .npmrc, and stays gated behind use_home_npmrc.
2765ecf to
0f8a0d2
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f8a0d2b62
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| rm .npmrc | ||
| _sedi 's#npmrc = "//:.npmrc",#use_home_npmrc = True,#' MODULE.bazel | ||
|
|
||
| # Replace the @aspect-build registry token with a tokenHelper that prints it instead, |
There was a problem hiding this comment.
Doesn't that remove the testing we were doing? I think we need to essentially clone this e2e test but use the new attribute?
There was a problem hiding this comment.
Restored that test to its _authToken form in 8e73269 and added a parallel e2e/npm_translate_lock_auth_token_helper that drives tokenHelper through the new npmrc_auth_file attribute. You're right - the in-place swap had quietly dropped the static-token coverage.
There was a problem hiding this comment.
Mind taking another look when you get a chance?
The tokenHelper path was folded into the existing npm_translate_lock_auth test by swapping its _authToken line, which dropped the _authToken coverage that test is there for. Restore that test and add a parallel e2e that drives tokenHelper through the new npmrc_auth_file attribute instead.
| `npmrc_auth_file`), never in the `npmrc` file. | ||
| """), | ||
| "npmrc_auth_file": attr.string(doc = """ | ||
| Path to the trusted user-level auth file to read when `use_home_npmrc` is True. |
There was a problem hiding this comment.
Why can't it be used without use_home_npmrc? Why can't it be a local path (which might then run/import something global or read a CI env var etc)?
| set -o errexit -o nounset -o pipefail | ||
|
|
||
| # Print the auth token on stdout. A real helper would mint or refresh a token here. | ||
| printf '%s\n' "${ASPECT_GH_PACKAGES_AUTH_TOKEN:-}" |
There was a problem hiding this comment.
So this depends on CI setting the variable... why do we need the sed logic in test.sh at all?
Can npm_translate_lock not reference this token-helper.sh directly and then the regular bazel test //... (which depends on CI setting the ASPECT_GH_PACKAGES_AUTH_TOKEN var) will work and we can drop test.sh?
3 participants
Adds support for pnpm's
tokenHelpernpmrc setting. AtokenHelperis an absolute path to an executable that prints an auth token on stdout - useful for registries that hand out short-lived tokens instead of a constant one. rules_js runs the helper and uses its output as the registry bearer token, so it takes the exact same path as a static//host/:_authToken=....The helper runs an arbitrary executable, so (like pnpm) it is only honored from the trusted user-level auth file, never a project
.npmrcchecked into the repo. That file is pnpm'snpmrcAuthFile, defaulting to~/.npmrc. You can point it elsewhere with the newnpmrc_auth_fileattribute or thePNPM_CONFIG_NPMRC_AUTH_FILEenv var (withNPM_CONFIG_USERCONFIGas an npm-style fallback). Reading it is still gated behinduse_home_npmrc, which stays the on/off switch.Both the global
tokenHelper=and per-registry//registry/:tokenHelper=forms work,${VAR}expansion applies to the path, and a helper takes precedence over a static_authTokenfor the same registry. A non-absolute path, a non-zero exit, or empty output fails the build with a clear message.This wires into the bzlmod extension, where the registry auth that feeds
npm_importis actually built.Changes are visible to end-users: yes
Support pnpm's
tokenHelpernpmrc setting. Withuse_home_npmrc, rules_js runs thetokenHelperexecutable for a registry and uses its output as the auth token, so you can authenticate with short-lived tokens. It is only honored from the trusted user-level auth file (default~/.npmrc, or setnpmrc_auth_file/PNPM_CONFIG_NPMRC_AUTH_FILE), never a project.npmrc.Test plan