From 03ac59a87708929e55ed7fdde466e9f1780e27b8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 28 Jan 2019 07:46:41 -0800
Subject: [PATCH] Don't put "spacePHID IN (...)" constraints in queries which
 will raise policy exceptions

Summary:
See T13240. Ref T13242. When we're issuing a query that will raise policy exceptions (i.e., give the user a "You Shall Not Pass" dialog if they can not see objects it loads), don't do space filtering in MySQL: when objects are filtered out in MySQL, we can't distinguish between "bad/invalid ID/object" and "policy filter", so we can't raise a policy exception.

This leads to cases where viewing an object shows "You Shall Not Pass" if you can't see it for any non-Spaces reason, but "404" if the reason is Spaces.

There's no product reason for this, it's just that `spacePHID IN (...)` is important for non-policy-raising queries (like a list of tasks) to reduce how much application filtering we need to do.

Test Plan:
Before:

```
$ git pull
phabricator-ssh-exec: No repository "spellbook" exists!
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
```

After:

```
$ git pull
phabricator-ssh-exec: [You Shall Not Pass: Unknown Object (Repository)] This object is in a space you do not have permission to access.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
```

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13242

Differential Revision: https://secure.phabricator.com/D20042
---
 .../policy/PhabricatorCursorPagedPolicyAwareQuery.php      | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
index 931840e8fb..9f7a69909a 100644
--- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
@@ -2855,6 +2855,13 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery
       }
     }
 
+    // See T13240. If this query raises policy exceptions, don't filter objects
+    // in the MySQL layer. We want them to reach the application layer so we
+    // can reject them and raise an exception.
+    if ($this->shouldRaisePolicyExceptions()) {
+      return null;
+    }
+
     $space_phids = array();
     $include_null = false;