From 366d33da91e24dfab2d748068d017cc33a66802d Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Sat, 15 Aug 2026 23:30:00 +0200 Subject: [PATCH 1/2] fix: throw a clear error when npm login runs without a TTY (#9860) --- lib/utils/auth.js | 8 ++++++ test/lib/utils/auth.js | 55 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 55e40d5c3c269..5c3c80dd06777 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -49,6 +49,14 @@ const login = async (npm, { creds, ...opts }) => { // auth type !== web or ENYI error w/ web login if (!res) { + if (!process.stdin.isTTY || !process.stdout.isTTY) { + throw Object.assign(new Error( + 'This command requires a TTY to prompt for a username and password.\n' + + 'Non-interactive auth is not supported for `npm login`.\n' + + 'Use `npm token create` or set an auth token in your .npmrc instead.' + ), { code: 'ENOTTY' }) + } + const username = await read.username('Username:', creds.username) const password = await read.password('Password:', creds.password) res = await otplease(npm, opts, (reqOpts) => loginCouch(username, password, reqOpts)) diff --git a/test/lib/utils/auth.js b/test/lib/utils/auth.js index 01d254ad8a0e7..a7ae7b5ff4e31 100644 --- a/test/lib/utils/auth.js +++ b/test/lib/utils/auth.js @@ -141,3 +141,58 @@ t.test('does not prompt if stdin or stdout is not a tty', async (t) => { }, }, fn), { message: 'nope' }, 'rejects with the original error') }) + +const setupLogin = async (t, { creds = {}, ...rest }, opts = {}) => { + const { login } = tmock(t, '{LIB}/utils/auth.js', { + '{LIB}/utils/read-user-info.js': { + username: async () => 'foo', + password: async () => 'bar', + }, + 'npm-profile': { + loginCouch: async () => ({ token: 'test-token' }), + }, + }) + const { npm } = await setupMockNpm(t, { + ...rest, + config: { 'auth-type': 'legacy', ...rest.config }, + }) + return login(npm, { creds, registry: 'https://registry.npmjs.org/', ...opts }) +} + +t.test('login throws a clear error when stdin is not a tty', async (t) => { + await t.rejects(setupLogin(t, { + globals: { + 'process.stdin': { isTTY: false }, + 'process.stdout': { isTTY: true }, + }, + }), { + code: 'ENOTTY', + message: /requires a TTY/, + }, 'rejects with a clear, actionable error instead of hanging') +}) + +t.test('login throws a clear error when stdout is not a tty', async (t) => { + await t.rejects(setupLogin(t, { + globals: { + 'process.stdin': { isTTY: true }, + 'process.stdout': { isTTY: false }, + }, + }), { + code: 'ENOTTY', + message: /requires a TTY/, + }, 'rejects with a clear, actionable error instead of hanging') +}) + +t.test('login succeeds with couch when stdin and stdout are ttys', async (t) => { + const result = await setupLogin(t, { + globals: { + 'process.stdin': { isTTY: true }, + 'process.stdout': { isTTY: true }, + }, + }) + + t.strictSame(result, { + message: 'Logged in on https://registry.npmjs.org/.', + newCreds: { token: 'test-token' }, + }) +}) From 8cd08833a081054d2ac84f816e94fab3cee0ce44 Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Sat, 15 Aug 2026 23:50:07 +0200 Subject: [PATCH 2/2] fix: address review feedback on login TTY guard (#9860) - fix mockLogin() in test/lib/commands/login.js to set isTTY: true on the mocked stdin/stdout PassThrough streams, which the new TTY guard was tripping on since PassThrough has no isTTY property; this had broken 4 pre-existing tests - add auth.js unit test coverage for the web-login -> ENYI -> couch fallback path, mocking loginWeb and open-url.js so the guard is exercised on that path too, not just legacy couch login - add a test where both stdin and stdout are non-TTY together - reword the error message so it no longer claims web login is unsupported non-interactively (it is); scope the message to the couch/legacy prompt path where a real TTY read is required - replace the circular 'npm token create' remediation with actionable steps: create a granular access token on npmjs.com, then set it via //registry.npmjs.org/:_authToken or NPM_TOKEN - rename the error code from ENOTTY (a real POSIX errno Node already uses for ioctl failures) to ENOTTYAUTH, and add a case for it in error-message.js so it renders with npm's standard summary/detail format instead of falling into the generic default handler --- lib/utils/auth.js | 6 +- lib/utils/error-message.js | 9 ++ .../test/lib/utils/error-message.js.test.cjs | 83 ++++++++++++------- test/lib/commands/login.js | 5 +- test/lib/utils/auth.js | 60 ++++++++++++-- test/lib/utils/error-message.js | 1 + 6 files changed, 123 insertions(+), 41 deletions(-) diff --git a/lib/utils/auth.js b/lib/utils/auth.js index 5c3c80dd06777..ac61b7aa6185d 100644 --- a/lib/utils/auth.js +++ b/lib/utils/auth.js @@ -51,10 +51,8 @@ const login = async (npm, { creds, ...opts }) => { if (!res) { if (!process.stdin.isTTY || !process.stdout.isTTY) { throw Object.assign(new Error( - 'This command requires a TTY to prompt for a username and password.\n' + - 'Non-interactive auth is not supported for `npm login`.\n' + - 'Use `npm token create` or set an auth token in your .npmrc instead.' - ), { code: 'ENOTTY' }) + 'npm login requires an interactive terminal to prompt for credentials.' + ), { code: 'ENOTTYAUTH' }) } const username = await read.username('Username:', creds.username) 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 a7ae7b5ff4e31..ea129ff21a0c7 100644 --- a/test/lib/utils/auth.js +++ b/test/lib/utils/auth.js @@ -142,14 +142,18 @@ t.test('does not prompt if stdin or stdout is not a tty', async (t) => { }, fn), { message: 'nope' }, 'rejects with the original error') }) -const setupLogin = async (t, { creds = {}, ...rest }, opts = {}) => { +const setupLogin = async (t, { creds = {}, loginWeb, ...rest }, opts = {}) => { const { login } = tmock(t, '{LIB}/utils/auth.js', { '{LIB}/utils/read-user-info.js': { username: async () => 'foo', password: async () => 'bar', }, + '{LIB}/utils/open-url.js': { + createOpener: () => () => {}, + }, 'npm-profile': { loginCouch: async () => ({ token: 'test-token' }), + ...(loginWeb ? { loginWeb } : {}), }, }) const { npm } = await setupMockNpm(t, { @@ -166,8 +170,8 @@ t.test('login throws a clear error when stdin is not a tty', async (t) => { 'process.stdout': { isTTY: true }, }, }), { - code: 'ENOTTY', - message: /requires a TTY/, + code: 'ENOTTYAUTH', + message: /requires an interactive terminal/, }, 'rejects with a clear, actionable error instead of hanging') }) @@ -178,8 +182,20 @@ t.test('login throws a clear error when stdout is not a tty', async (t) => { 'process.stdout': { isTTY: false }, }, }), { - code: 'ENOTTY', - message: /requires a TTY/, + code: 'ENOTTYAUTH', + message: /requires an interactive terminal/, + }, 'rejects with a clear, actionable error instead of hanging') +}) + +t.test('login throws a clear error when neither stdin nor stdout is a tty', async (t) => { + await t.rejects(setupLogin(t, { + globals: { + 'process.stdin': { isTTY: false }, + 'process.stdout': { isTTY: false }, + }, + }), { + code: 'ENOTTYAUTH', + message: /requires an interactive terminal/, }, 'rejects with a clear, actionable error instead of hanging') }) @@ -196,3 +212,37 @@ t.test('login succeeds with couch when stdin and stdout are ttys', async (t) => newCreds: { token: 'test-token' }, }) }) + +t.test('login throws a clear error for the web login ENYI fallback when not a tty', async (t) => { + await t.rejects(setupLogin(t, { + config: { 'auth-type': 'web' }, + loginWeb: async () => { + throw Object.assign(new Error('web login not supported'), { code: 'ENYI' }) + }, + globals: { + 'process.stdin': { isTTY: false }, + 'process.stdout': { isTTY: false }, + }, + }), { + code: 'ENOTTYAUTH', + message: /requires an interactive terminal/, + }, 'rejects with a clear, actionable error instead of hanging on the couch fallback') +}) + +t.test('login falls back to couch after web login ENYI when a 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' }) + }, + globals: { + 'process.stdin': { isTTY: true }, + 'process.stdout': { isTTY: 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',