From def19bb8de1e2e3f33aa5ba19bb1c9dd4d4a8d67 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 12 Mar 2012 10:34:37 -0700 Subject: [PATCH] Add additional protections against local repository misconfigurations Summary: We've hit a couple of these in the wild, raise better error messages when the local repo is toast / broken / nonsense. Test Plan: Broke my local repo in all of the different ways we test for, verified I got an error message in each case. Reviewers: btrahan, abirchall Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T964, T924 Differential Revision: https://secure.phabricator.com/D1855 --- .../PhabricatorRepositoryGitFetchDaemon.php | 51 +++++++++++++++++++ .../repository/daemon/gitfetch/__init__.php | 2 + 2 files changed, 53 insertions(+) diff --git a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php b/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php index 74349961c4..cc42b714dc 100644 --- a/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php +++ b/src/applications/repository/daemon/gitfetch/PhabricatorRepositoryGitFetchDaemon.php @@ -37,6 +37,57 @@ final class PhabricatorRepositoryGitFetchDaemon PhabricatorRepository $repository, $local_path) { + // Run a bunch of sanity checks to detect people checking out repositories + // inside other repositories, making empty directories, pointing the local + // path at some random file or path, etc. + + list($err, $stdout) = $repository->execLocalCommand( + 'rev-parse --show-toplevel'); + + if ($err) { + + // Try to raise a more tailored error message in the more common case + // of the user creating an empty directory. (We could try to remove it, + // but might not be able to, and it's much simpler to raise a good + // message than try to navigate those waters.) + if (is_dir($local_path)) { + $files = Filesystem::listDirectory($local_path, $include_hidden = true); + if (!$files) { + throw new Exception( + "Expected to find a git repository at '{$local_path}', but there ". + "is an empty directory there. Remove the directory: the daemon ". + "will run 'git clone' for you."); + } + } + + throw new Exception( + "Expected to find a git repository at '{$local_path}', but there is ". + "a non-repository directory (with other stuff in it) there. Move or ". + "remove this directory (or reconfigure the repository to use a ". + "different directory), and then either clone a repository yourself ". + "or let the daemon do it."); + } else { + $repo_path = rtrim($stdout, "\n"); + + if (empty($repo_path)) { + throw new Exception( + "Expected to find a git repository at '{$local_path}', but ". + "there was no result from `git rev-parse --show-toplevel`. ". + "Something is misconfigured or broken. The git repository ". + "may be inside a '.git/' directory."); + } + + if (!Filesystem::pathsAreEquivalent($repo_path, $local_path)) { + throw new Exception( + "Expected to find repo at '{$local_path}', but the actual ". + "git repository root for this directory is '{$repo_path}'. ". + "Something is misconfigured. The repository's 'Local Path' should ". + "be set to some place where the daemon can check out a working ". + "copy, and should not be inside another git repository."); + } + } + + // This is a local command, but needs credentials. $future = $repository->getRemoteCommandFuture('fetch --all --prune'); $future->setCWD($local_path); diff --git a/src/applications/repository/daemon/gitfetch/__init__.php b/src/applications/repository/daemon/gitfetch/__init__.php index d59679cb9a..59c22675a8 100644 --- a/src/applications/repository/daemon/gitfetch/__init__.php +++ b/src/applications/repository/daemon/gitfetch/__init__.php @@ -9,5 +9,7 @@ phutil_require_module('phabricator', 'applications/repository/constants/repositorytype'); phutil_require_module('phabricator', 'applications/repository/daemon/pulllocal'); +phutil_require_module('phutil', 'filesystem'); + phutil_require_source('PhabricatorRepositoryGitFetchDaemon.php');