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

[FAST_BUILD] No sudo when run with rootless triplet #2132

Merged
merged 4 commits into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions images/docker-stacks-foundation/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,14 @@ if [ "$(id -u)" == 0 ]; then
unset_explicit_env_vars

_log "Running as ${NB_USER}:" "${cmd[@]}"
exec sudo --preserve-env --set-home --user "${NB_USER}" \
LD_LIBRARY_PATH="${LD_LIBRARY_PATH}" \
PATH="${PATH}" \
PYTHONPATH="${PYTHONPATH:-}" \
"${cmd[@]}"
if [ "${NB_USER}" = "root" ] && [ "${NB_UID}" = "$(id -u "${NB_USER}")" ] && [ "${NB_GID}" = "$(id -g "${NB_USER}")" ]; then
HOME="/home/root" exec "${cmd[@]}"
else
exec sudo --preserve-env --set-home --user "${NB_USER}" \
LD_LIBRARY_PATH="${LD_LIBRARY_PATH}" \
PATH="${PATH}" \
PYTHONPATH="${PYTHONPATH:-}" \
"${cmd[@]}"
# Notes on how we ensure that the environment that this container is started
# with is preserved (except vars listed in JUPYTER_ENV_VARS_TO_UNSET) when
# we transition from running as root to running as NB_USER.
Expand Down Expand Up @@ -187,6 +190,7 @@ if [ "$(id -u)" == 0 ]; then
# above in /etc/sudoers.d/path. Thus PATH is irrelevant to how the above
# sudo command resolves the path of `${cmd[@]}`. The PATH will be relevant
# for resolving paths of any subprocesses spawned by `${cmd[@]}`.
fi

# The container didn't start as the root user, so we will have to act as the
# user we started as.
Expand Down
39 changes: 39 additions & 0 deletions tests/docker-stacks-foundation/test_user_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,3 +305,42 @@ def test_startsh_multiple_exec(container: TrackedContainer) -> None:
"WARNING: start.sh is the default ENTRYPOINT, do not include it in CMD"
in warnings[0]
)


def test_rootless_triplet_change(container: TrackedContainer) -> None:
"""Container should change the username (`NB_USER`), the UID and the GID of the default user."""
logs = container.run_and_wait(
timeout=10,
tty=True,
user="root",
environment=["NB_USER=root", "NB_UID=0", "NB_GID=0"],
command=["id"],
)
assert "uid=0(root)" in logs
assert "gid=0(root)" in logs
assert "groups=0(root)" in logs


def test_rootless_triplet_home(container: TrackedContainer) -> None:
"""Container should change the home directory for triplet NB_USER=root, NB_UID=0, NB_GID=0."""
logs = container.run_and_wait(
timeout=10,
tty=True,
user="root",
environment=["NB_USER=root", "NB_UID=0", "NB_GID=0"],
command=["bash", "-c", "echo HOME=${HOME} && getent passwd root"],
)
assert "HOME=/home/root" in logs
assert "root:x:0:0:root:/home/root:/bin/bash" in logs


def test_rootless_triplet_sudo(container: TrackedContainer) -> None:
"""Container should not be started with sudo for triplet NB_USER=root, NB_UID=0, NB_GID=0."""
logs = container.run_and_wait(
timeout=10,
tty=True,
user="root",
environment=["NB_USER=root", "NB_UID=0", "NB_GID=0"],
command=["env"],
)
assert "SUDO" not in logs
Copy link
Member

Choose a reason for hiding this comment

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

Why is this in capital letters?

Copy link
Contributor Author

@benz0li benz0li Aug 11, 2024

Choose a reason for hiding this comment

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

To check if any of

SUDO_GID=0
SUDO_COMMAND=/usr/local/bin/start-notebook.py
SUDO_USER=root
SUDO_UID=0

is set in the environment.

That is the case, when the container is started with sudo – but should not in this case.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks!