diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 55e40d5c3c269..fb0e7577d81c1 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -32,6 +32,34 @@ const otplease = async (npm, opts, fn) => { } } +// `read` only settles its returned promise on `line`, `error`, or `SIGINT` — when +// stdin hits EOF without ever producing a line (an empty/redirected file, a pipe +// that closes without writing, or Ctrl-D on a real TTY), readline emits `close` +// with no handler for it, and the promise never settles (npm/cli#9860). Race the +// prompt against stdin's own `end` event so that case rejects with a clear error +// instead of hanging forever, while leaving every other case — a real TTY session, +// or genuine piped/redirected credentials that actually produce a line before EOF — +// unaffected, since `prompt` then wins the race and this listener is a no-op. +const readOrErrorOnStdinEnd = (prompt) => new Promise((resolve, reject) => { + const cleanup = () => process.stdin.removeListener('end', onStdinEnd) + const onStdinEnd = () => { + cleanup() + reject(Object.assign(new Error( + 'npm login requires an interactive terminal, or credentials piped to stdin, to authenticate.' + ), { code: 'ENOTTYAUTH' })) + } + process.stdin.once('end', onStdinEnd) + prompt + .then((value) => { + cleanup() + return resolve(value) + }) + .catch((err) => { + cleanup() + reject(err) + }) +}) + const login = async (npm, { creds, ...opts }) => { const authType = npm.config.get('auth-type') let res @@ -49,8 +77,8 @@ const login = async (npm, { creds, ...opts }) => { // auth type !== web or ENYI error w/ web login if (!res) { - const username = await read.username('Username:', creds.username) - const password = await read.password('Password:', creds.password) + const username = await readOrErrorOnStdinEnd(read.username('Username:', creds.username)) + const password = await readOrErrorOnStdinEnd(read.password('Password:', creds.password)) res = await otplease(npm, opts, (reqOpts) => loginCouch(username, password, reqOpts)) } diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index 41e35bd78ac46..6b8d354a125f8 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -271,6 +271,15 @@ const errorMessage = (er, npm) => { detail.push(['need auth', 'You need to authorize this machine using `npm login`']) break + case 'ENOTTYAUTH': + summary.push(['need auth', er.message]) + detail.push(['need auth', [ + 'Create a granular access token at https://www.npmjs.com/, then set it with:', + ' npm config set //registry.npmjs.org/:_authToken=', + 'or by setting the NPM_TOKEN environment variable.', + ].join('\n')]) + break + case 'ECONNRESET': case 'ENOTFOUND': case 'ETIMEDOUT': diff --git a/tap-snapshots/test/lib/utils/error-message.js.test.cjs b/tap-snapshots/test/lib/utils/error-message.js.test.cjs index a63412c96ea4a..1eb1d9d7244fb 100644 --- a/tap-snapshots/test/lib/utils/error-message.js.test.cjs +++ b/tap-snapshots/test/lib/utils/error-message.js.test.cjs @@ -1188,6 +1188,28 @@ Object { ` exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 13`] = ` +Object { + "detail": Array [ + Array [ + "network", + String( + This is a problem related to network connectivity. + In most cases you are behind a proxy or have bad network settings. + + If you are behind a proxy, please make sure that the 'proxy' config is set properly. See: 'npm help config' + ), + ], + ], + "summary": Array [ + Array [ + "network", + "foo", + ], + ], +} +` + +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 14`] = ` Object { "detail": Array [ Array [ @@ -1212,7 +1234,7 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 14`] = ` +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 15`] = ` Object { "detail": Array [ Array [ @@ -1232,7 +1254,7 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 15`] = ` +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 16`] = ` Object { "detail": Array [ Array [ @@ -1249,7 +1271,7 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 16`] = ` +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 17`] = ` Object { "detail": Array [ Array [ @@ -1270,26 +1292,6 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 17`] = ` -Object { - "detail": Array [ - Array [ - "typeerror", - String( - This is an error with npm itself. Please report this error at: - https://github.com/npm/cli/issues - ), - ], - ], - "summary": Array [ - Array [ - "typeerror", - "dummy stack trace", - ], - ], -} -` - exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 18`] = ` Object { "detail": Array [ @@ -1372,6 +1374,26 @@ Object { ` exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 21`] = ` +Object { + "detail": Array [ + Array [ + "typeerror", + String( + This is an error with npm itself. Please report this error at: + https://github.com/npm/cli/issues + ), + ], + ], + "summary": Array [ + Array [ + "typeerror", + "dummy stack trace", + ], + ], +} +` + +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 22`] = ` Object { "detail": Array [ Array [ @@ -1388,7 +1410,7 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 22`] = ` +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 23`] = ` Object { "detail": Array [ Array [ @@ -1405,7 +1427,7 @@ Object { } ` -exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 23`] = ` +exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 24`] = ` Object { "detail": Array [ Array [ @@ -1569,18 +1591,17 @@ exports[`test/lib/utils/error-message.js TAP just simple messages > must match s Object { "detail": Array [ Array [ - "network", + "need auth", String( - This is a problem related to network connectivity. - In most cases you are behind a proxy or have bad network settings. - - If you are behind a proxy, please make sure that the 'proxy' config is set properly. See: 'npm help config' + Create a granular access token at https://www.npmjs.com/, then set it with: + npm config set //registry.npmjs.org/:_authToken= + or by setting the NPM_TOKEN environment variable. ), ], ], "summary": Array [ Array [ - "network", + "need auth", "foo", ], ], diff --git a/test/lib/commands/login.js b/test/lib/commands/login.js index 623bc5845708f..dec8fdfb0d458 100644 --- a/test/lib/commands/login.js +++ b/test/lib/commands/login.js @@ -12,12 +12,15 @@ const mockLogin = async (t, { stdin: stdinLines, registry: registryUrl, ...optio let stdin if (stdinLines) { stdin = new stream.PassThrough() + stdin.isTTY = true for (const l of stdinLines) { stdin.write(l + '\n') } + const stdout = new stream.PassThrough() // to quiet readline + stdout.isTTY = true mockGlobals(t, { 'process.stdin': stdin, - 'process.stdout': new stream.PassThrough(), // to quiet readline + 'process.stdout': stdout, }, { replace: true }) } const mock = await loadMockNpm(t, { diff --git a/test/lib/utils/auth.js b/test/lib/utils/auth.js index 01d254ad8a0e7..3ca6d10b3f7e0 100644 --- a/test/lib/utils/auth.js +++ b/test/lib/utils/auth.js @@ -1,4 +1,6 @@ const t = require('tap') +const { EventEmitter } = require('node:events') +const mockGlobals = require('@npmcli/mock-globals') const setupMockNpm = require('../../fixtures/mock-npm') const tmock = require('../../fixtures/tmock') @@ -141,3 +143,137 @@ t.test('does not prompt if stdin or stdout is not a tty', async (t) => { }, }, fn), { message: 'nope' }, 'rejects with the original error') }) + +// A real EventEmitter, not a plain `{ isTTY }` object: the fix races the prompt +// against stdin's own `end` event, so the mock needs working `.once`/`.removeListener` +// the way a real stream (TTY or piped) would have. +const mockStdin = (isTTY) => { + const stdin = new EventEmitter() + stdin.isTTY = isTTY + return stdin +} + +// A promise that never settles — the real `read()` genuinely hangs forever on +// EOF-without-a-line (that's the bug), so this is a more faithful mock than one +// that resolves after some number of microtask ticks. It also sidesteps having +// to time `stdin.emit('end')` against an artificial resolution: with a prompt +// that can never resolve on its own, only the `end` event can ever settle the +// race, so `end` can be emitted after any delay, on any code path (legacy or +// the web-login ENYI fallback, which has an extra await before reaching the +// prompt), without a risk of a flaky win/lose depending on exact tick count. +const hangingRead = () => new Promise(() => {}) + +const setupLogin = (t, params, opts = {}) => { + const { creds = {}, loginWeb, stdin = mockStdin(true), hangs, ...rest } = params + const { login } = tmock(t, '{LIB}/utils/auth.js', { + '{LIB}/utils/read-user-info.js': { + username: hangs ? hangingRead : async () => 'foo', + password: hangs ? hangingRead : async () => 'bar', + }, + '{LIB}/utils/open-url.js': { + createOpener: () => () => {}, + }, + 'npm-profile': { + loginCouch: async () => ({ token: 'test-token' }), + ...(loginWeb ? { loginWeb } : {}), + }, + }) + return setupMockNpm(t, { + ...rest, + config: { 'auth-type': 'legacy', ...rest.config }, + }).then(({ npm }) => { + // process.stdin/process.stdout are lazily-defined getters, not plain + // assignable properties — mock-npm's own `globals` option merges them in + // without `{ replace: true }`, which silently fails to swap them. Using + // mockGlobals directly here (like test/lib/commands/login.js's mockLogin + // does) is required for `process.stdin` inside login() to actually be + // this test's `stdin` object. + mockGlobals(t, { + 'process.stdin': stdin, + 'process.stdout': { isTTY: true }, + ...rest.globals, + }, { replace: true }) + const result = login(npm, { creds, registry: 'https://registry.npmjs.org/', ...opts }) + if (hangs) { + // Give login() a full event-loop turn to reach and register the + // `end` listener (needed for the web-login ENYI fallback, which awaits + // the rejected loginWeb() call first) before ending stdin. + setImmediate(() => stdin.emit('end')) + } + return result + }) +} + +t.test('login throws a clear error when stdin ends before answering', async (t) => { + await t.rejects(setupLogin(t, { + stdin: mockStdin(false), + hangs: true, + }), { + code: 'ENOTTYAUTH', + message: /requires an interactive terminal, or credentials piped/, + }, 'rejects with a clear, actionable error instead of hanging') +}) + +t.test('login throws a clear error when a real tty session ends (e.g. Ctrl-D) before answering', async (t) => { + await t.rejects(setupLogin(t, { + stdin: mockStdin(true), + hangs: true, + }), { + code: 'ENOTTYAUTH', + message: /requires an interactive terminal, or credentials piped/, + }, 'rejects instead of hanging even though stdin is a tty') +}) + +t.test('login succeeds with couch on a real tty', async (t) => { + const result = await setupLogin(t, { + stdin: mockStdin(true), + }) + + t.strictSame(result, { + message: 'Logged in on https://registry.npmjs.org/.', + newCreds: { token: 'test-token' }, + }) +}) + +t.test('login succeeds with couch when credentials are piped and stdin is not a tty', async (t) => { + // Not passing hangs: the mocked username/password resolve on + // their own, standing in for "the pipe produced a line" — the fix must not + // reject this just because isTTY is false. + const result = await setupLogin(t, { + stdin: mockStdin(false), + }) + + t.strictSame(result, { + message: 'Logged in on https://registry.npmjs.org/.', + newCreds: { token: 'test-token' }, + }) +}) + +t.test('login throws a clear error for the web login ENYI fallback when stdin ends before answering', async (t) => { + await t.rejects(setupLogin(t, { + config: { 'auth-type': 'web' }, + loginWeb: async () => { + throw Object.assign(new Error('web login not supported'), { code: 'ENYI' }) + }, + stdin: mockStdin(false), + hangs: true, + }), { + code: 'ENOTTYAUTH', + message: /requires an interactive terminal, or credentials piped/, + }, 'rejects with a clear, actionable error instead of hanging on the couch fallback') +}) + +t.test('login falls back to couch after web login ENYI on a real tty', async (t) => { + const result = await setupLogin(t, { + config: { 'auth-type': 'web' }, + loginWeb: async () => { + throw Object.assign(new Error('web login not supported'), { code: 'ENYI' }) + }, + stdin: mockStdin(true), + }) + + t.strictSame(result, { + message: 'Logged in on https://registry.npmjs.org/.', + newCreds: { token: 'test-token' }, + }) +}) diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 44a57eca645d7..f188befce6eca 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -59,6 +59,7 @@ t.test('just simple messages', async t => { 'EISGIT', 'EEXIST', 'ENEEDAUTH', + 'ENOTTYAUTH', 'ECONNRESET', 'ENOTFOUND', 'ETIMEDOUT',