commit 05eb2d093cb11e63cafa96a84368f1ce11d33c15
author Lorenzo Colitti <lorenzo@google.com> Fri Jun 17 15:09:50 2022 +0900
committer Lorenzo Colitti <lorenzo@google.com> Fri Jun 17 15:13:04 2022 +0900
tree 841c23635cd839b55488fceb72e9e2b8c57e0544
parent 8274622cb4e4a4486f44db6b94e1c0067506dd97

Refuse to adopt shell permissions if they were already adopted.

This does not work because when the permissions are dropped, all
previously-adopted permissions will be dropped as well. Attempts
to do so will likely result in hard-to-understand test failures.

On S+, where we can detect this, fail with a clear error message.

Test: treehugger
Bug: 225092753
Change-Id: I1587ab77f885c595f0b9168c6739303c6227218e

staticlibs/testutils/devicetests/com/android/testutils/TestPermissionUtil.kt

diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestPermissionUtil.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestPermissionUtil.kt
index f557f18..a4dbd9a 100644
--- a/staticlibs/testutils/devicetests/com/android/testutils/TestPermissionUtil.kt
+++ b/staticlibs/testutils/devicetests/com/android/testutils/TestPermissionUtil.kt
@@ -19,6 +19,7 @@
 package com.android.testutils
 
 import androidx.test.platform.app.InstrumentationRegistry
+import com.android.modules.utils.build.SdkLevel
 import com.android.testutils.ExceptionUtils.ThrowingRunnable
 import com.android.testutils.ExceptionUtils.ThrowingSupplier
 
@@ -31,6 +32,23 @@
  */
 fun <T> runAsShell(vararg permissions: String, task: () -> T): T {
     val autom = InstrumentationRegistry.getInstrumentation().uiAutomation
+
+    // Calls to adoptShellPermissionIdentity do not nest, and dropShellPermissionIdentity drops all
+    // permissions. Thus, nesting calls will almost certainly cause test bugs, On S+, where we can
+    // detect this, refuse to do it.
+    //
+    // TODO: when R is deprecated, we could try to make this work instead.
+    // - Get the list of previously-adopted permissions.
+    // - Adopt the union of the previously-adopted and newly-requested permissions.
+    // - Run the task.
+    // - Adopt the previously-adopted permissions, dropping the ones just adopted.
+    //
+    // This would allow tests (and utility classes, such as the TestCarrierConfigReceiver attempted
+    // in aosp/2106007) to call runAsShell even within a test that has already adopted permissions.
+    if (SdkLevel.isAtLeastS() && !autom.getAdoptedShellPermissions().isNullOrEmpty()) {
+        throw IllegalStateException("adoptShellPermissionIdentity calls must not be nested")
+    }
+
     autom.adoptShellPermissionIdentity(*permissions)
     try {
         return task()