Skip to content

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

Open
wakqasahmed wants to merge 2 commits into
npm:latestfrom
wakqasahmed:fix/login-tty-guard-9860
Open

fix: throw a clear error when npm login runs without a TTY (#9860)#9878
wakqasahmed wants to merge 2 commits into
npm:latestfrom
wakqasahmed:fix/login-tty-guard-9860

Conversation

@wakqasahmed

Copy link
Copy Markdown

Fixes #9860

Summary

npm login currently prompts for username/password via read, which reads
from stdin. When stdin (or stdout) is not a TTY — e.g. running npm login in
CI, a script, or with input piped in — the prompt has nothing to read from and
the command exits with code 1 and no error message, giving the user no idea
what went wrong.

This adds an explicit TTY guard in login() (lib/utils/auth.js), mirroring
the existing TTY check already used in otplease() in the same file. When
process.stdin.isTTY or process.stdout.isTTY is falsy, npm login now
throws a clear, actionable error instead of silently failing:

This command requires a TTY to prompt for a username and password.
Non-interactive auth is not supported for `npm login`.
Use `npm token create` or set an auth token in your .npmrc instead.

The error is thrown before any prompt is attempted, and only affects the
couch-login fallback path (i.e. after web login is skipped or not
applicable) — npm login --auth-type=web behavior is unchanged.

Tests

Added to test/lib/utils/auth.js:

  • login throws a clear error when stdin is not a tty
  • login throws a clear error when stdout is not a tty
  • login succeeds with couch when stdin and stdout are ttys (regression guard)

Ran locally: tap test/lib/utils/auth.js --no-coverage — all 10 tests pass.
Also ran eslint on both changed files with no errors.

@wakqasahmed
wakqasahmed requested review from a team as code owners August 15, 2026 21:30

@wakqasahmed wakqasahmed left a comment

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.

Cold-start review (no prior context on this change).

Verdict: changes required — this breaks the existing test suite

The TTY guard itself is placed correctly and matches the otplease() / openUrlPrompt() precedent exactly (!process.stdin.isTTY || !process.stdout.isTTY), and the web-login path is genuinely unaffected. But test/lib/commands/login.js fails completely on this branch — 4 pre-existing tests now throw ENOTTY, and the file aborts. This was not caught because only test/lib/utils/auth.js was run.

Reproduction

git clone --branch fix/login-tty-guard-9860 <fork> && npm i --ignore-scripts
./node_modules/.bin/tap test/lib/commands/login.js --no-coverage

On this branch:

not ok 1 - basic login
not ok 2 - scoped login default registry
not ok 3 - scoped login scoped registry
not ok 4 - fallback            (web -> ENYI -> couch)
not ok - test/lib/commands/login.js

Each fails with:

not ok 1 - This command requires a TTY to prompt for a username and password. ...
  code: ENOTTY

With the guard hunk reverted and nothing else changed, the same file is green (ok 1 - test/lib/commands/login.js). So the failures are caused by this PR, not by the environment.

Cause: mockLogin() in test/lib/commands/login.js does

mockGlobals(t, {
  'process.stdin': stdin,                          // stream.PassThrough
  'process.stdout': new stream.PassThrough(),
}, { replace: true })

Neither PassThrough has isTTY, so it is undefined and the new guard fires before read.username() is ever reached. (Even without replace, process.stdin.isTTY is falsy under tap/CI, so these would fail regardless of the mock.) Every existing test in that file that drives login through the couch prompt must be updated to set isTTY: true on the mocked globals, the same way the otplease tests in test/lib/utils/auth.js already do.

Note that web > fallback is in that list — that is exactly repro #2 from #9860, and it is the one existing test that covers the ENYI fallback. It has to keep passing with isTTY: true, not be deleted.

On the parent question: does the web-login path have the same untested bug?

No — I checked and the web path is safe, for a reason worth stating in the PR body since it is not obvious:

  • loginWeb() reaches createOpener() -> openUrlPrompt() in lib/utils/open-url.js, which already has if (browser === false || !process.stdin.isTTY || !process.stdout.isTTY) { return }. It prints the URL and returns instead of prompting, so npm login --auth-type=web still works non-interactively. This PR does not regress that.
  • Non-interactive CI auth via NPM_TOKEN / _authToken in .npmrc never enters lib/utils/auth.js login() at all, so it is unaffected.
  • read.username(msg, default) always prompts on the first call even when creds.username is pre-populated (the early return is guarded by isRetry), so there is no pre-seeded-credentials flow that used to work non-interactively and now breaks.

So the only real behavioural change for users is: an interactive stdin with a redirected stdout (npm login > log) now hard-fails. That is a change, but it is exactly what otplease() and openUrlPrompt() already do, so I think consistency wins here — just be aware of it.

Scope note

This is a UX guard, not a root-cause fix. read() still never settles on stdin EOF, so npm profile set password, npm profile enable-2fa and npm token create (the other read-user-info.js consumers) still hang the same way. That is fine for this PR's scope, but the upstream npm/read fix referenced in #9860 is still the durable fix and this should not be presented as superseding it.

Individual points inline.

Comment thread lib/utils/auth.js

// auth type !== web or ENYI error w/ web login
if (!res) {
if (!process.stdin.isTTY || !process.stdout.isTTY) {

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.

The predicate matches the precedent in otplease() at line 10 and openUrlPrompt() in lib/utils/open-url.js exactly, and the placement inside if (!res) is right — it covers both --auth-type=legacy and the web -> ENYI -> couch fallback, which are the two repros in #9860, while leaving --auth-type=web alone. No objection to the guard itself.

The blocker is fallout: test/lib/commands/login.js mocks process.stdin/process.stdout as stream.PassThrough with { replace: true } and never sets isTTY, so this guard fires in legacy > basic login, legacy > scoped login default registry, legacy > scoped login scoped registry and web > fallback. All four fail with code: ENOTTY and the file aborts. Reverting just this hunk makes the file green again, so it is this change.

Fix is in the test fixture, not here: mockLogin() should add isTTY: true to the replaced stdin/stdout globals (a PassThrough happily takes the property), matching how the otplease tests in test/lib/utils/auth.js already pass { isTTY: true }. Please run tap test/lib/commands/login.js as well as test/lib/utils/auth.js before pushing.

Comment thread lib/utils/auth.js Outdated
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' +

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.

Wording is inaccurate in the case this guard most often fires. "Non-interactive auth is not supported for npm login" is not true: npm login --auth-type=web is usable without a TTY — openUrlPrompt() deliberately skips the prompt and prints the URL for you to open elsewhere. And the ENYI fallback means a user who ran the web flow can land on this exact error, at which point the message tells them something they just did successfully is unsupported.

Suggest scoping the claim to the thing that is actually unsupported, e.g.

`npm login` needs a TTY to prompt for a username and password.

and leaving the remedies to the detail lines.

Comment thread lib/utils/auth.js Outdated
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.'

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.

npm token create is circular advice here: it authenticates against the registry using credentials the user does not have, which is why they are running npm login in the first place. Someone in CI with no TTY and no token cannot act on this.

The actionable remedies are: create a granular access token on npmjs.com, then set //registry.npmjs.org/:_authToken=... in .npmrc or NPM_TOKEN in the environment. Worth naming the config key explicitly — it is the part people get wrong.

Two smaller conventions points on this block:

  1. ENOTTY is a real POSIX errno that Node emits for genuine ioctl failures, so overloading it slightly muddies npm error code ENOTTY. ENEEDAUTH already exists in lib/utils/error-message.js and is semantically adjacent; either reuse it or pick a clearly npm-specific code.
  2. There is no case for this code in lib/utils/error-message.js, so all three lines fall into the default branch and land in summary — the user gets three npm error lines of equal weight. npm's convention (see ENEEDAUTH, EACCES, ENOSPC) is a one-line summary.push plus the remediation in detail.push. Adding a small case there would make this read like the rest of npm's errors.

Comment thread test/lib/utils/auth.js
})
const { npm } = await setupMockNpm(t, {
...rest,
config: { 'auth-type': 'legacy', ...rest.config },

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.)

Comment thread test/lib/utils/auth.js
}, 'rejects with a clear, actionable error instead of hanging')
})

t.test('login succeeds with couch when stdin and stdout are ttys', async (t) => {

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.

Good that a positive-path regression guard is included — asserting that a real TTY still logs in is the thing that stops a guard like this from quietly disabling the feature.

One gap: there is no case for stdin and stdout both non-TTY. The two negative tests each flip only one flag, so a future refactor that changed || to && would still pass both of them. The existing does not prompt if stdin or stdout is not a tty test above sets both to false; mirroring that here would close it.

- 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
@wakqasahmed

Copy link
Copy Markdown
Author

Thanks for the thorough review — addressed everything:

Blocking: broken test suite — fixed. mockLogin() in test/lib/commands/login.js now sets isTTY: true on both mocked stdin/stdout PassThrough streams (they had no isTTY at all, so the new guard was firing on tests that legitimately simulate an interactive session). legacy > basic login, legacy > scoped login default registry, legacy > scoped login scoped registry, and web > fallback all pass again — confirmed with tap test/lib/commands/login.js --no-coverage (13/13 green, no aborts).

Web-login path test coverage gap — fixed. Extended setupLogin() in test/lib/utils/auth.js to optionally mock loginWeb and open-url.js's createOpener, and added two new tests: the ENYI→couch fallback now hits ENOTTYAUTH when non-TTY, and still succeeds via couch when a real TTY is present. This exercises repro #2 from the issue, not just the legacy couch path.

Missing both-non-TTY test case — added (login throws a clear error when neither stdin nor stdout is a tty), so an ||&& regression in the guard would be caught.

Inaccurate error message — reworded to npm login requires an interactive terminal to prompt for credentials., scoped specifically to the couch/legacy prompt path (web login is unaffected and was never really the thing failing).

Circular npm token create advice — replaced with actionable steps: create a granular access token on npmjs.com, then set it via //registry.npmjs.org/:_authToken=<token> or the NPM_TOKEN env var.

Error code collision — renamed ENOTTY (a real Node/POSIX errno used elsewhere for ioctl failures) to ENOTTYAUTH, and added a proper case 'ENOTTYAUTH' in lib/utils/error-message.js with a summary/detail pair so it renders in npm's standard error format instead of falling into the generic default handler. Updated the snapshot test accordingly.

Re-ran both test/lib/utils/auth.js (17 tests) and test/lib/commands/login.js (13 tests) — all green, no regressions. Also ran test/lib/utils/error-message.js since I touched that file — all green. eslint clean on all changed files.

Left as-is, per your note: this remains a UX guard, not a fix for the underlying read()-never-settles-on-EOF issue — npm profile/npm token still hang the same way, and the durable fix is the separate upstream npm/read#157. Not claiming otherwise here.

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

Successfully merging this pull request may close these issues.

[BUG] npm login exits 1 with no error message when stdin is not interactive

1 participant