Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERR_INVALID_AUTH triggered by semantic-release npm despite not having this field set in checkout directory #777

Closed
cdaringe opened this issue Apr 12, 2024 · 11 comments

Comments

@cdaringe
Copy link

cdaringe commented Apr 12, 2024

Problem

Unable to release due to a common auth error regarding the _auth field which is not present in my config.

The associated stack:

09:32:27 [4:32:27 PM] [semantic-release] › ✘  An error occurred while running semantic-release: Error: Command failed with exit code 1: npm version 1.0.0 --userconfig /tmp/c1b619531cbf95a121902fabb9d6d9f9/.npmrc --no-git-tag-version --allow-same-version
09:32:27 npm ERR! code ERR_INVALID_AUTH
09:32:27 npm ERR! Invalid auth configuration found: `_auth` must be renamed to `//npme.foo.com/:_auth` in user config
09:32:27 npm ERR! Please run `npm config fix` to repair your configuration.`
09:32:27 
09:32:27 npm ERR! A complete log of this run can be found in: /mnt/jenkinspan/workspace/myorg/foolocale/.npm/_logs/2024-04-12T16_32_27_233Z-debug-0.log
09:32:27     at makeError (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/execa@8.0.1/node_modules/execa/lib/error.js:60:11)
09:32:27     at handlePromise (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/execa@8.0.1/node_modules/execa/index.js:124:26)
09:32:27     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
09:32:27     at async default (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/@semantic-release+npm@12.0.0_semantic-release@23.0.8/node_modules/@semantic-release/npm/lib/prepare.js:26:3)
09:32:27     at async prepare (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/@semantic-release+npm@12.0.0_semantic-release@23.0.8/node_modules/@semantic-release/npm/index.js:63:3)
09:32:27     at async validator (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/semantic-release@23.0.8_typescript@5.4.5/node_modules/semantic-release/lib/plugins/normalize.js:36:24)
09:32:27     at async file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/semantic-release@23.0.8_typescript@5.4.5/node_modules/semantic-release/lib/plugins/pipeline.js:38:36
09:32:27     at async Promise.all (index 0)
09:32:27     at async next (file:///mnt/jenkinspan/workspace/myorg/foolocale/ws/node_modules/.pnpm/p-reduce@3.0.0/node_modules/p-reduce/index.js:15:44) {
09:32:27   shortMessage: 'Command failed with exit code 1: npm version 1.0.0 --userconfig /tmp/c1b619531cbf95a121902fabb9d6d9f9/.npmrc --no-git-tag-version --allow-same-version',
09:32:27   command: 'npm version 1.0.0 --userconfig /tmp/c1b619531cbf95a121902fabb9d6d9f9/.npmrc --no-git-tag-version --allow-same-version',
09:32:27   escapedCommand: 'npm version 1.0.0 --userconfig "/tmp/c1b619531cbf95a121902fabb9d6d9f9/.npmrc" --no-git-tag-version --allow-same-version',
09:32:27   exitCode: 1,

Research

Observation 1: The error claims _auth format is wrong, but my npm config has no _auth set. npm config list in my checkout in CI shows no _auth (evidence attached below).

  • my local .npmrc has only one entry (registry), nothing to do with auth
  • global prefixes globalconfig does have //npme.foo.com/:_auth=(redacted) in it. It does not have _auth!
npm config list, immediately before calling semantic release
09:32:21 + npm config list
09:32:22 ; "global" config from /mnt/jenkinspan/workspace/myorg/foolocale/tools/nix_64/nodejs-21.7.3/globalconfig
09:32:22 
09:32:22 //npme.foo.com/:_auth = (protected) 
09:32:22 email = "devtools@myorg.com" 
09:32:22 prefix = "/mnt/jenkinspan/workspace/myorg/foolocale/tools/nix_64/nodejs-21.7.3" 
09:32:22 
09:32:22 ; "project" config from /mnt/jenkinspan/workspace/myorg/foolocale/ws/.npmrc
09:32:22 
09:32:22 ; registry = "https://npme.foo.com" ; overridden by env
09:32:22 
09:32:22 ; "env" config from environment
09:32:22 
09:32:22 cache = "/mnt/jenkinspan/workspace/myorg/foolocale/.npm" 
09:32:22 registry = "https://npme.foo.com/" 
09:32:22 store-dir = "/mnt/jenkinspan/workspace/myorg/foolocale/.pnpm" 
09:32:22 userconfig = "/mnt/jenkinspan/workspace/myorg/foolocale/tools/nix_64/nodejs-21.7.3/userconfig" 
09:32:22 
09:32:22 ; node bin location = /mnt/jenkinspan/workspace/myorg/foolocale/tools/nix_64/nodejs-21.7.3/bin/node
09:32:22 ; node version = v21.7.3
09:32:22 ; npm local prefix = /mnt/jenkinspan/workspace/myorg/foolocale/ws
09:32:22 ; npm version = 10.2.4
09:32:22 ; cwd = /mnt/jenkinspan/workspace/myorg/foolocale/ws
09:32:22 ; HOME = /mnt/jenkinspan
09:32:22 ; Run `npm config ls -l` to show all defaults.

Observation 2: The command fails on npm version, per the stack trace, but the same npm version invocation works when run locally in my CI checkout, vs the /tmp/... semrel uses.

npm version 0.0.3 --userconfig ./.npmrc --no-git-tag-version --allow-same-version
09:32:22 + npm version 0.0.3 --userconfig ./.npmrc --no-git-tag-version --allow-same-version
09:32:22 
09:32:22 v0.0.3

Discussion

  • why would npm version work in CI immediately before semantic-release, but not when sem-rel does stuff off in a temp dir?
@travi
Copy link
Member

travi commented Apr 12, 2024

What versions are you using of semantic-release and the npm plugin?

@cdaringe
Copy link
Author

cdaringe commented Apr 12, 2024 via email

@cdaringe
Copy link
Author

cdaringe commented Apr 12, 2024

Ok, i've learned some more.

  • in my working checkout dir, npm config list reads data from <workspace>/.npmrc and my <prefix>/globalconfig.
  • when sem-rel processes, it thinks my configs are <workspace>/.npmrc and ../../<workspace>/.npmrc, which is a grantparent. More specifically:
13:04:37   "configs": [
13:04:37     "/mnt/jenkinspan/.npmrc", # this config file is not processed by npm list config when run in my project checkout
13:04:37     "/mnt/jenkinspan/workspace/myorg/foo/ws/.npmrc"
13:04:37   ]
// set-npmrc-auth.js
import rc from "rc";

rc();

tldr, sem-rels use of rc() writes an incompatible npm config with my system, where if it used just my checkout's npm config, it would work successfully. rc does not appear to match the way npm sources configuration files? still digging.

My intuition says that this config generation method creates opporitunity for desync.

Instead of reading files and checking for valid auth, we should seriously consider querying npm to produce it's full configuration. We're we to have captured npm config list as the source of config before calling verify-auth, it would have said "ya, this system is auth ready." However, instead of consulting npm, we tried to re-create what npm does, but it was not aligned, and can lead to two different failures:

  1. first, reading in a previously unread file (which corrupted my config), and second,
  2. not consulting the prefix/global config file (which dropped my critical, correct auth config!)

@travi
Copy link
Member

travi commented Apr 12, 2024

tldr, sem-rels use of rc() writes an incompatible npm config with my system

the npm plugin used to write a config that use the _auth approach that your error describes. this is because it used to be a valid approach. however, we dropped support for that approach in v10.0.0 of the npm plugin once npm dropped support. this is why i was curious about what versions you were using.

since you are using a version that is significantly newer than that change, it does not directly explain your problem. does the version you mentioned above match the one in the release workflow output? this feels like you have another (older) version of semantic-release involved somewhere.

also, are NPM_USERNAME and NPM_PASSWORD provided to your release workflow as environment variables?

@travi
Copy link
Member

travi commented Apr 12, 2024

Instead of reading files and checking for valid auth, we should seriously consider querying npm to produce it's full configuration. We're we to have captured npm config list as the source of config before calling verify-auth, it would have said "ya, this system is auth ready." However, instead of consulting npm, we tried to re-create what npm does, but it was not aligned, and can lead to two different failures:

  1. first, reading in a previously unread file (which corrupted my config), and second,
  2. not consulting the prefix/global config file (which dropped my critical, correct auth config!)

i wont disagree that there is opportunity for improvement, but there are reasons that the auth process is how it is today. our goal is to leverage npm config as much as possible, but it is important to understand that auth is expected to be provided in the NPM_TOKEN environment variable. if this is done as expected, semantic-release will produce a valid config that factors in the remaining config from your project .npmrc with auth injected correctly.

@cdaringe
Copy link
Author

cdaringe commented May 3, 2024

npm plugin once npm dropped support ... does the version you mentioned above match the one

The error message is directly from sem-rel running of the latest version of npm (latest at the time of writing). And indeed, I can assert latest versions of all sem-rel packages were being used. The failure mode is/was isolated to the latest version of the npm sem-rel plugin (this package).

This package can generate an incorrect .npmrc, used in its temdir. My CI system had an old, incorrect evil floating .npmrc file on disk somewhere in a parent folder, which I did not place, nor do I service as a user of this CI system. I talked with the CI admin to remove it. NPM_TOKEN or no NPM_TOKEN, this package has code which concats all parent npmrcs which is no longer correct behavior, ref https://docs.npmjs.com/cli/v7/configuring-npm/npmrc#files

I think we're aligned on that, but re-expressing just for clarity sake.

but it is important to understand ... if this is done as expected

This is incorrect. The following is all stated with courtesy. This lib currently supports checking for _auth, and can fail incorrectly due to bad concat'ing. "done as expected" is surprising language given the state of the code. The lib supports this now despite the unsupported claim:

const currentConfig = configs ? (await Promise.all(configs.map((config) => fs.readFile(config)))).join("\n") : "";
if (getAuthToken(registry, { npmrc: rcConfig })) {
await fs.outputFile(npmrc, currentConfig);
return;
}

Even if I had set NPM_TOKEN, that failing code block would have triggered and my publish failed due to the rc concatenating in a dead rc file. FWIW, I do rely on my auth being provided by my CI providing team, so NPM_TOKEN isn't available to me, which is... a thing 😵 .

It's a subtle and nuanced failure mode, no doubt.

@travi
Copy link
Member

travi commented May 3, 2024

My CI system had an old, incorrect evil floating .npmrc file on disk somewhere in a parent folder, which I did not place, nor do I service as a user of this CI system. I talked with the CI admin to remove it.

does this mean that the trouble you faced is resolved in your case? if so, that gives us a bit of space to gather the details more clearly for improving the future handling of the configs

NPM_TOKEN or no NPM_TOKEN, this package has code which concats all parent npmrcs which is no longer correct behavior, ref docs.npmjs.com/cli/v7/configuring-npm/npmrc#files

as i mentioned, our goal is to align with npm as closely as possible, but we know we are currently not fully in alignment. appreciate you highlighting the details that you found that resulted in misalignment and surprise. i'll open an issue to track this separately

This is incorrect. The following is all stated with courtesy.

honestly appreciate the persistence to get the right details in the open. i'll be honest, i've never provided auth through an .npmrc when using semantic-release, so i forgot that we supported that approach officially. for a long time, the env var was the only supported method of providing a token. i understand the cases where it fits how some teams prefer to configure CI servers, but i honestly thought we hadnt added that support yet. i can say with certainty that we no longer inject auth as _auth since that was removed as part of #437

This lib currently supports checking for _auth, and can fail incorrectly due to bad concat'ing.

i'm still a little bit confused here, so i'm hoping to just clarify some specifics. since it has been a bit since i've explored this particular part of our implementation for a while, i looked a little more closely. i see that registry-auth-token does find and return a token from an .npmrc file that uses the legacy unscoped approach. what i'm not clear about from your statement is what you mean about "bad concat'ing". are you saying that the approach of combining the files is introducing a syntax or formatting error, that the _auth property should be stripped, or is this referring to your point that some found files should not be concat'ed because of the mismatch with default npm behvaior?

@travi
Copy link
Member

travi commented May 3, 2024

i'll open an issue to track this separately

#790

@cdaringe
Copy link
Author

cdaringe commented May 8, 2024

does this mean that the trouble you faced is resolved in your case?

Yep!

honestly appreciate the persistence ... since #437

ya no problem.

what i'm not clear about from your statement is what you mean about "bad concat'ing".

Indeed, I should have used more precise language. I intended to mean "[referring my point] that some found files should not be concat'ed because of the mismatch with default npm behavior".

Edit: I realized you did grok my point above 😵 , so my expansion below wasn't needed :)

Let me lay it out a bit more clearly.
  1. We use the rc lib to take a best guess at what the npm config is here:
    const { configs, ...rcConfig } = rc(
    This is in fact the problem, because it returned an array of configs that included a .npmrc that npm itself would not include.
  2. This line
    logger.log("Reading npm config from %s", configs.join(", "));
    printed that naughty npm config file that npm itself would not have loaded.
  3. We then concatenated it and used it
    const currentConfig = configs ? (await Promise.all(configs.map((config) => fs.readFile(config)))).join("\n") : "";

...so, npm complained and said "@cdaringe, my friend, this is bad _auth" and i said to it, "sir! sir! surely you are mistaken!" But it was not mistaken 😄

@travi
Copy link
Member

travi commented May 8, 2024

got it. that makes sense.

I realized you did grok my point above 😵 , so my expansion below wasn't needed :)

i do appreciate the confirmation so that i know for sure we are on the same page now

i agree that we should address this, and i think i have this covered in the additional issues created as a result of this thread. since you have a path forward with the current state of things, would it be fair to close this issue in favor of the others to address aligning better with current npm behavior?

@cdaringe
Copy link
Author

cdaringe commented May 8, 2024

Awesome, ya let's close!

@cdaringe cdaringe closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants