Node: os.userInfo().shell is '' if no shell in /etc/passwd

Created on 29 Sep 2017  路  4Comments  路  Source: nodejs/node

  • Version: master
  • Platform: Linux (Ubuntu 14.04)
  • Subsystem: os

Continuation of https://github.com/libuv/libuv/issues/1570

Basically this line fails:

https://github.com/nodejs/node/blob/4843c2f41571088b16e673c1f996bc361ab526a6/test/parallel/test-os.js#L196

because os.userInfo().shell is ''. This is because os.userInfo() calls uv__getpwuid_r() which calls getpwuid_r(3), which reads the relevant line from /etc/passwd:

gib:x:1234:3210:Gibson Fahnestock:/home/gib:

However the user shell on this machine is not defined in /etc/passwd, so we get a default.

The question is whether we should just update the test (remove this check, so don't actually check that os.userInfo().shell actually returns anything, or whether os.userInfo().shell should handle this case.

I guess it comes down to whether this function's purpose is just "read the shell from /etc/passwd", or "tell you which shell the user is using". If it's the former we remove the test, if it's the latter we should check either the default shell, or process.env.SHELL.

The default shell is either just defaulting to /bin/sh, or it's coming from /etc/adduser.conf:

# The DSHELL variable specifies the default login shell on your system
DSHELL=/bin/bash
os

Most helpful comment

A few comments. First, the os.userInfo() docs state:

on POSIX platforms, this is typically a subset of the password file

So, I don't think it would be unreasonable to require that the information be in the password file.

remove this check, so don't actually check that os.userInfo().shell actually returns anything

The test could still check that a string is returned. It could even go further and perform the current check if the string has a length > 0.

if it's the latter we should check either the default shell, or process.env.SHELL

I'm not saying that we should go this route, but we already do something like this with the user's home directory. os.userInfo() returns the home directory from the password file, while os.homedir() takes a few environment variables into account.

All 4 comments

A few comments. First, the os.userInfo() docs state:

on POSIX platforms, this is typically a subset of the password file

So, I don't think it would be unreasonable to require that the information be in the password file.

remove this check, so don't actually check that os.userInfo().shell actually returns anything

The test could still check that a string is returned. It could even go further and perform the current check if the string has a length > 0.

if it's the latter we should check either the default shell, or process.env.SHELL

I'm not saying that we should go this route, but we already do something like this with the user's home directory. os.userInfo() returns the home directory from the password file, while os.homedir() takes a few environment variables into account.

This is potentially an Ubuntu/Debian behaviour.

At least, the typical Ubuntu user entry, based on (default) adduser creation, results in an empty shell specified in the /etc/passwd file (no shell specifically defined). Ubuntu's man page specifies that the agent _setting_ the SHELL environment variable is login; where empty, the value will default to /bin/sh.

So, it seems it is possible to have an interactive shell without the export of SHELL on Ubuntu systems.

typical Ubuntu user entry, based on (default) adduser creation, results in an empty shell specified in the /etc/passwd file

Unrelated, but that sounds like a bug in adduser to me.

I'm not saying that we should go this route, but we already do something like this with the user's home directory. os.userInfo() returns the home directory from the password file, while os.homedir() takes a few environment variables into account.

That makes sense, so if people need something higher level to get the user's default shell we can just add an os.shell().

Was this page helpful?
0 / 5 - 0 ratings

Related issues

danielstaleiny picture danielstaleiny  路  3Comments

stevenvachon picture stevenvachon  路  3Comments

vsemozhetbyt picture vsemozhetbyt  路  3Comments

Icemic picture Icemic  路  3Comments

sandeepks1 picture sandeepks1  路  3Comments