From ca78c1825a665c8c3a37cc10331e8300c40e3e2a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 11 Aug 2016 16:02:57 -0700 Subject: [PATCH] When already running as the daemon user, don't "sudo" daemon commands 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 --- .../daemon/PhabricatorDaemon.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/infrastructure/daemon/PhabricatorDaemon.php b/src/infrastructure/daemon/PhabricatorDaemon.php index 6fd2ed6016..f42a59f134 100644 --- a/src/infrastructure/daemon/PhabricatorDaemon.php +++ b/src/infrastructure/daemon/PhabricatorDaemon.php @@ -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');