nodejs / docker-node

Official Docker Image for Node.js :whale: :turtle: :rocket:
https://hub.docker.com/_/node/
MIT License
8.27k stars 1.97k forks source link

fix: enable non-root corepack usage #1992

Closed eckdanny closed 11 months ago

eckdanny commented 1 year ago

Description

Makes gid=1000 (the non-root user node's group id) the owning group of /usr/local/bin. This enables the non-root user to perform operations like corepack {enable,prepare,...} without permission errors.

Closes 1732.

Testing Details

manual: (see Q about testing in checklist section)

cd <major>/<variant>
docker build -t test .
docker run --rm -it -u 1000 -w /home/node test bash

node@3da2bc76ca5b:~$ corepack enable
node@3da2bc76ca5b:~$ corepack prepare 'pnpm@8' --activate
Preparing pnpm@8 for immediate activation...
node@3da2bc76ca5b:~$ pnpm --version
8.10.0

Example Output(if appropriate)

Types of changes

Checklist

tests? I don't see how to run a suite

meyfa commented 1 year ago

I would advocate against this change. While it may solve the problem, it compromises security by allowing a non-root user access to a directory that should be reserved for root.

eckdanny commented 1 year ago

I think this would best solved at image build-time by adding following cmd to Dockerfile(s):

chgrp 1000 /usr/local/bin && chmod g+w /usr/local/bin

I maintain a suite of private org-level images which extend docker.io/_/node:{18,20}, and that's what I did.

Proper ACLs (via setfacl) would be probably be best, but unlikely that'll be available for all targets/upstreams. Unless maintainers are opposed to granting group=node write perms to /usr/local/bin, i don't think there's a simpler way.

@meyfa If you know of a better way, happy to resubmit

eckdanny commented 1 year ago

After symlinking into $HOME/bin, corepack {prepare,use,...} commands behave exactly how we'd like for the non-root user. Playing with the Dockerfile-debian.template a bit:

   && rm yarn-v$YARN_VERSION.tar.gz.asc yarn-v$YARN_VERSION.tar.gz \
+  && NODE_HOME="$(getent passwd node | cut -d: -f6)" \
+  && mkdir -p $NODE_HOME/bin && ln -s "$(which corepack)" $NODE_HOME/bin/corepack \
+  && echo 'export PATH="$HOME/bin:$PATH"' >>$NODE_HOME/.bashrc && chown -R node:node $NODE_HOME/bin \
  # smoke test
  && yarn --version

achieves desired outcome. (The PATH precedence stuff is already in /home/node/.profile but the entrypoint is not a login shell so that's why I append to .bashrc)

Thoughts?

eckdanny commented 11 months ago
'ing dir(s) in the /usr/local tree _is_ a hacky workaround. It'd be better/cleaner/better/\*nix-FSH-compliant to solve another way. Other alternative impls I tried (which i also didn't like) 1. symlink'ing corepack to `$HOME/bin` in entrypoint or bash startup file(s) ensure for `uid=1000` has `$PATH` precedence:
Over combination of {[non-]interactive shell,[non-]login shell,[non-]bash shell,[non-]debian,...} this gets hard really fast, if not impossible for some folks like Jenkins CI Docker Pipeline users (like me) where additional docker {run,exec} args aren't in a public API 1. use `corepack --install-directory ...` all over the place if uuid!=0:
works but what a pain. 💡 It'd be great if `corepack` had an user-level config file or there was an ENV_VAR which could set, but I didn't see anything like that in docs 1. mutilate the entrypoint with lots of conditionals to test for uuid=1000, test for corepack dirs for writability, set `$PATH`, exec, set --, _etc_:
pretty awful this one For my use cases of docker.io/_/node (base for ephemeral build environments only) I'm ok w/ adding this hack **downstream**, but don't think it should be used here. If `corepack` was "pre-enabled" by uuid=0 at image buildtime, that'd be OK. Then uuid=1000 (node) can activate/install things with corepack, but can't turn it off