Skip to content

fix: throw a clear error when npm login runs without a TTY (#9860) #9878

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.

Sign up for GitHub

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

Closed
wakqasahmed wants to merge 3 commits into from
Closed

fix: throw a clear error when npm login runs without a TTY (#9860) #9878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
366d33d
fix: throw a clear error when npm login runs without a TTY (#9860)
wakqasahmed Aug 15, 2026
8cd0883
fix: address review feedback on login TTY guard (#9860)
wakqasahmed Aug 15, 2026
830ac57
fix(login): race stdin's own end event instead of gating on isTTY (#9…
wakqasahmed Aug 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions lib/utils/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
}

Expand Down
9 changes: 9 additions & 0 deletions lib/utils/error-message.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=<token>',
'or by setting the NPM_TOKEN environment variable.',
].join('\n')])
break

case 'ECONNRESET':
case 'ENOTFOUND':
case 'ETIMEDOUT':
Expand Down
83 changes: 52 additions & 31 deletions tap-snapshots/test/lib/utils/error-message.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand All @@ -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 [
Expand All @@ -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 [
Expand All @@ -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 [
Expand All @@ -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 [
Expand Down Expand Up @@ -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 [
Expand All @@ -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 [
Expand All @@ -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 [
Expand Down Expand Up @@ -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=<token>
or by setting the NPM_TOKEN environment variable.
),
],
],
"summary": Array [
Array [
"network",
"need auth",
"foo",
],
],
Expand Down
5 changes: 4 additions & 1 deletion test/lib/commands/login.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
136 changes: 136 additions & 0 deletions test/lib/utils/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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')

Expand Down Expand Up @@ -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 },

@wakqasahmed wakqasahmed Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forcing 'auth-type': 'legacy' here is what makes the helper work, but it also means this helper structurally cannot test the branch #9860 actually reports as repro #2 — web login, registry returns 4xx, npm-profile maps it to ENYI, npm falls back to couch and hits the prompt.

It is not just the config: the npm-profile mock only stubs loginCouch, so loginWeb and webAuthOpener are undefined, and {LIB}/utils/open-url.js is not mocked at all. Set auth-type to web and you get loginWeb is not a function, not an ENYI fallback — so the guard's most important real-world entry point is silently untested and any future regression in the fallback branch would not be caught here.

Worth extending the helper rather than leaving the gap:

'{LIB}/utils/open-url.js': { createOpener: () => () => {} },
'npm-profile': {
  loginWeb: async () => { throw Object.assign(new Error('nyi'), { code: 'ENYI' }) },
  loginCouch: async () => ({ token: 'test-token' }),
},

plus a case with config: { 'auth-type': 'web' } and non-TTY globals asserting ENOTTY, and one asserting a non-ENYI loginWeb error still rethrows untouched rather than being swallowed into the TTY message.

(For what it is worth, test/lib/commands/login.js already has a web > fallback integration test covering that path — it is one of the four this PR breaks. Fixing its isTTY mocks is the higher-priority half of this.)

}).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' },
})
})
1 change: 1 addition & 0 deletions test/lib/utils/error-message.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ t.test('just simple messages', async t => {
'EISGIT',
'EEXIST',
'ENEEDAUTH',
'ENOTTYAUTH',
'ECONNRESET',
'ENOTFOUND',
'ETIMEDOUT',
Expand Down