@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
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
+5
-6
+5
-6
src/applications/diffusion/request/DiffusionRequest.php
+5
-6
src/applications/diffusion/request/DiffusionRequest.php
···
387
387
if (empty($this->repositoryCommit)) {
388
388
$repository = $this->getRepository();
389
389
390
-
// TODO: (T603) This should be a real query, but we need to sort out
391
-
// the viewer.
392
-
$commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
393
-
'repositoryID = %d AND commitIdentifier = %s',
394
-
$repository->getID(),
395
-
$this->getStableCommit());
390
+
$commit = id(new DiffusionCommitQuery())
391
+
->setViewer($this->getUser())
392
+
->withRepositoryIDs(array($repository->getID()))
393
+
->withIdentifiers(array($this->getStableCommit()))
394
+
->executeOne();
396
395
if ($commit) {
397
396
$commit->attachRepository($repository);
398
397
}