Skip to content

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

2 participants