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

fix(entrypoint): set non-bogus $HOME when using su to avoid 3rd-party issues #2318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Oct 19, 2024

Fixes #2317
Fixes #2053
Fixes #1288

We preserve the environment - because we need it - when executing commands:

run_as() {
if [ "$(id -u)" = 0 ]; then
su -p "$user" -s /bin/sh -c "$1"
else
sh -c "$1"
fi
}

This doesn't cause issues typically, but since $HOME is carried over from root it can cause issues like #2317 / #2053 / #1288 that are challenging to diagnose.

It is kind of ugly that we carry over a bogus $HOME value. Since the path is also inaccessible, it's also pointless.

We might consider instead one of the following approaches:

  • Whitelist instead of preserve the entire environment in run_as?
    • using -p, switching to -l with a whitelist (-w): https://man.archlinux.org/man/su.1.en#OPTIONS
    • extra work for each variable we change
    • may cause other side effects (such as breaking non-image variables) since admins pass variables in for other purposes we can't predict/known ahead of time
  • Set HOME to the correct value
    • prepending HOME=/var/www to the su call
    • easy, targeted fix

This PR takes the second approach.

This should cut down on problems people encounter when running up against third-party tools that query $HOME. Since the value was already problematic, I can't think of any problems this will cause. It should not be a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant