Skip to content
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.

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

Failed write to /etc/passwd #1520

Closed
maresb opened this issue Nov 10, 2021 · 4 comments · Fixed by #1546
Closed

Failed write to /etc/passwd #1520

maresb opened this issue Nov 10, 2021 · 4 comments · Fixed by #1546

Comments

@maresb
Copy link
Contributor

maresb commented Nov 10, 2021

From @bilke:

@maresb Does this part got lost?

I am asking because the following does not work anymore:

$ docker run --rm -p 8888:8888 -v $PWD:/home/jovyan/work --user `id -u $USER` \
    --group-add users my_image

Running: start.sh jupyter lab
id: cannot find name for user ID 40841
WARNING: container must be started as root to change the desired user's name with NB_USER!
WARNING: container must be started as root to change the desired user's id with NB_UID!
WARNING: container must be started as root to change the desired user's group id with NB_GID!
There is no entry in /etc/passwd for our UID. Attempting to fix...
Renaming old jovyan user to nayvoj (1000:100)
sed: couldn't open temporary file /etc/sedAELey6: Permission denied

Before removal of this section it printed:

Adding passwd file entry for 40841

Originally posted by @bilke in #1512 (comment)

@maresb
Copy link
Contributor Author

maresb commented Nov 10, 2021

Thanks @bilke for the quick report. We made some huge changes to start.sh, and and unfortunately they had an unintended side affect.

We need to add some test cases for your scenario to avoid breaking this in the future.

Old code:

        # User is not attempting to override user/group via environment
        # variables, but they could still have overridden the uid/gid that
        # container runs as. Check that the user has an entry in the passwd
        # file and if not add an entry.
        STATUS=0 && whoami &> /dev/null || STATUS=$? && true
        if [[ "${STATUS}" != "0" ]]; then
            if [[ -w /etc/passwd ]]; then
                echo "Adding passwd file entry for $(id -u)"
                sed -e "s/^jovyan:/nayvoj:/" /etc/passwd > /tmp/passwd
                echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /tmp/passwd
                cat /tmp/passwd > /etc/passwd
                rm /tmp/passwd
            else
                echo 'Container must be run with group "root" to update passwd file'
            fi
        fi

New code:

    # Attempt to ensure the user uid we currently run as has a named entry in
    # the /etc/passwd file, as it avoids software crashing on hard assumptions
    # on such entry. Writing to the /etc/passwd was allowed for the root group
    # from the Dockerfile during build.
    #
    # ref: https://github.com/jupyter/docker-stacks/issues/552
    if ! whoami &> /dev/null; then
        echo "There is no entry in /etc/passwd for our UID. Attempting to fix..."
        if [[ -w /etc/passwd ]]; then
            echo "Renaming old jovyan user to nayvoj ($(id -u jovyan):$(id -g jovyan))"
            sed --in-place "s/^jovyan:/nayvoj:/" /etc/passwd

            echo "jovyan:x:$(id -u):$(id -g):,,,:/home/jovyan:/bin/bash" >> /etc/passwd
            echo "Added new jovyan user ($(id -u):$(id -g)). Fixed UID!"
        else
            echo "WARNING: unable to fix missing /etc/passwd entry because we don't have write permission."
        fi
    fi

We switched from using sed on a temporary file to using sed --inplace, but unfortunately it seems that sed --inplace requires write permissions on the directory.

I'll propose a quick fix...

@maresb
Copy link
Contributor Author

maresb commented Nov 10, 2021

@bilke, once the new image is built and uploaded the problem should be fixed. The false warnings are still present for the moment.

@mathbunnyru
Copy link
Member

The new image is built and works as intended.
@maresb when you have time, please, get rid of false warnings and add the test for this case.

I will keep this issue open to not to forget to add a test case and fix the warnings.

@bilke
Copy link

bilke commented Nov 11, 2021

I can confirm that the image is working again! Thanks a lot!

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 a pull request may close this issue.

3 participants