From 8573d5b0c1a33483f352166c38cee3f3202365c4 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Sun, 1 Feb 2015 09:33:12 -0800 Subject: [PATCH] Policy - lock down loadCommit() from DiffusionRequest objects Summary: Ref T7094. The class DiffusionRequest has other public methods which use getUser() in an unguarded way. Code inspection of the call sites for loadCommit() also leads me to believe the $user is properly set. Test Plan: clicked around diffusion a bunch and everything seemed to work okay. (happy to test any particular esoteric endpoints that come to mind) Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7094 Differential Revision: https://secure.phabricator.com/D11585 --- .../diffusion/request/DiffusionRequest.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 9e804a54b5..8e51a4f509 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -387,12 +387,11 @@ abstract class DiffusionRequest { if (empty($this->repositoryCommit)) { $repository = $this->getRepository(); - // TODO: (T603) This should be a real query, but we need to sort out - // the viewer. - $commit = id(new PhabricatorRepositoryCommit())->loadOneWhere( - 'repositoryID = %d AND commitIdentifier = %s', - $repository->getID(), - $this->getStableCommit()); + $commit = id(new DiffusionCommitQuery()) + ->setViewer($this->getUser()) + ->withRepositoryIDs(array($repository->getID())) + ->withIdentifiers(array($this->getStableCommit())) + ->executeOne(); if ($commit) { $commit->attachRepository($repository); }