Page MenuHomePhorge

$HOME missing from commands issued by ExecFuture
Open, NormalPublic

Description

As far as I can see, it seems to me that every command issued by the ExecFuture system, does not have the $HOME environment variable.

Documentation:

https://we.phorge.it/book/arcanist/class/ExecFuture/

Among other things, this is a problem in the Config page (/config) since it issues git log but without an $HOME it cannot parse any .gitconfig and cannot read safe.directory and so it fails.

Questions:

  1. Is $HOME missing by design in ExecFuture? I think no.
  2. Should we enable $HOME as default? I think yes.

Tested with:

  1. It does NOT work with Apache mod_php PHP 7.4 variables_order = "GPCS" (default)
  2. It does NOT work with apache mod_php PHP 7.4 variables_order = "EGPCS"
  3. ...

Interestingly, the Config page start working with this patch I crafted, to manually create an HOME environment variable in this way:

      $log_futures[$lib] = id(new ExecFuture('%C', $log_command))
+       ->setEnv(array('HOME' => posix_getpwuid(posix_getuid())['dir']))
        ->setCWD($root);

The above workaround was tested in Linux with mod_php and it's probably too much system-centric. This specific HOME-related sub-problem is probably better to be discussed here:

T15298: Get the HOME of the user running the PHP webserver

Event Timeline

valerio.bozzolan triaged this task as Normal priority.
valerio.bozzolan created this object in space S1 Public.

Could you check the server configuration of php.ini for the value of variable_orders? This open task on phab suggests that on servers it should enforce including E causing phab's environment to be passed to sub-commands, but does not want to mess with it on users' machines.

https://secure.phabricator.com/T12071

In T15281#7096, @speck wrote:

Could you check the server configuration of php.ini for the value of variable_orders? This open task on phab suggests that on servers it should enforce including E causing phab's environment to be passed to sub-commands, but does not want to mess with it on users' machines.

https://secure.phabricator.com/T12071

Yup for some reasons it seems with or without that "E" in variables_order the $HOME is not passed to underlying Future commands

In upstream phabricator I worked with Evan to update the Mercurial API in a way that all mercurial commands/futures are executed through a common path and allow making modifications to only those executions. For this we should look at something similar so that we're only passing $HOME to git commands and not to others.

https://secure.phabricator.com/D21697 is the change I'm thinking of, and might be useful as reference

Thank you speck, very interesting.

I agree and I think that D25149 follows your suggestion of just exposing an $HOME environment variable, only to the specific commands needing that.

Feel free to take a quick look to tell if I'm misusing Futures

(Ouch I couldn't figure out where HOME comes from by looking at the mentioned diff in secure dot)

Ok, looks like my personal install is missing $HOME as well, so I can probably try to reproduce.

In T12071, @epriestly mentions that individually selecting env-vars to copy "build[s] some resistance to "Shellshock" class vulnerabilities", which is kind of a compelling argument.

See https://we.phorge.it/T15533#11839 - the env-selecting code in ExecFuture has this logic:

E \ custom envyesno
have Ecustom + copy envCopy Env
no Ecustom + 2 copy important envvarsEmpty env
  • "custom env" means "setEnv() was called".
  • There's also a way to specify "custom env" without copying any env vars, but I hope it's not used.

All executions in the form of exec*() count as "no custom env".

I think the "empty env" scenario is not really good, and that there are a few more important envvars (LC_*, HOME) we can copy in the non-E case (Or just require E, but see previous comment about that).

I wasn't able to track down the logic behind this - I guess E used to always be enabled, and then it doesn't really matter?