revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00
Code Issues Releases Wiki Activity

When already running as the daemon user, don't "sudo" daemon commands

Browse source 
Summary:
The cluster synchronization code runs either actively (before returning a response to `git clone`, for example) or passively (routinely, as the daemons update reposiories).

The active sync runs as the web user (if running `git clone http://...`) or the VCS user (if running `git clone ssh://...`). But the passive sync runs as the daemon user.

All of these sync processes need to run actual commands as the daemon user (`git fetch ...`).

For the active ones, we must `sudo`.

For the passive ones, we're already the right user. We run the same code, and end up trying to sudo to ourselves, which `sudo` isn't happy about by default.

Depending on how `sudo` is configured and which users things are running as this might work anyway, but it's silly and if it doesn't work it requires you to go make non-obvious, weird config changes that are unintuitive and somewhat nonsensical. This is probably worse on the balance than adding a bit of complexity to the code.

Instead, test which user we're running as. If it's already the right user, don't sudo.

Test Plan:
  - Ran `bin/repository update --trace` as daemon user, saw no more `sudo`.
  - Ran a `git clone` to make sure that didn't break.

Reviewers: chad, avivey

Reviewed By: avivey

Differential Revision: https://secure.phabricator.com/D16391
This commit is contained in:
epriestley 2016-08-11 16:02:57 -07:00
parent 39d4e21eec
commit ca78c1825a
1 changed files with 18 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/infrastructure/daemon/PhabricatorDaemon.php
View file

@ -34,6 +34,24 @@ abstract class PhabricatorDaemon extends PhutilDaemon {
return $command;
}
// We may reach this method while already running as the daemon user: for
// example, active and passive synchronization of clustered repositories
// run the same commands through the same code, but as different users.
// By default, `sudo` won't let you sudo to yourself, so we can get into
// trouble if we're already running as the daemon user unless the host has
// been configured to let the daemon user run commands as itself.
// Since this is silly and more complicated than doing this check, don't
// use `sudo` if we're already running as the correct user.
if (function_exists('posix_getuid')) {
$uid = posix_getuid();
$info = posix_getpwuid($uid);
if ($info && $info['name'] == $user) {
return $command;
}
}
// Get the absolute path so we're safe against the caller wiping out
// PATH.
$sudo = Filesystem::resolveBinary('sudo');