Fix the work profile the package list drops and the module that cannot reach it

daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt

@@ -357,7 +357,10 @@ object VectorService : IVectorDaemon.Stub() {
               !intent.getBooleanExtra(Intent.EXTRA_REPLACING, false) &&
               moduleName != null) {
 
-            ModuleDatabase.modulesIncludingNewApps().forEach { xposedModule ->
+            // Only the modules auto-including in the user this app arrived in. Asked without
+            // one, an app installed in a work profile landed in the scope of a module configured
+            // only for the owner.
+            ModuleDatabase.modulesIncludingNewApps(userId).forEach { xposedModule ->
               val scopeList = ModuleDatabase.getModuleScope(xposedModule) ?: mutableListOf()
 
               val newScope =

daemon/src/main/kotlin/org/matrix/vector/daemon/data/Database.kt

@@ -8,7 +8,7 @@ import java.io.File
 import org.matrix.vector.daemon.utils.FakeContext
 
 private const val TAG = "VectorDatabase"
-private const val DB_VERSION = 4
+private const val DB_VERSION = 5
 
 class Database(context: Context? = FakeContext()) :
     SQLiteOpenHelper(context, FileSystem.dbPath.absolutePath, null, DB_VERSION) {
@@ -30,12 +30,10 @@ class Database(context: Context? = FakeContext()) :
             module_pkg_name text NOT NULL UNIQUE,
             apk_path text NOT NULL,
             enabled BOOLEAN DEFAULT 0 CHECK (enabled IN (0, 1)),
-            -- `include new apps` everywhere else: when set, a package installed from now on is
-            -- added to this module's scope. The column keeps the older name on purpose. SQLite
-            -- gained ALTER TABLE RENAME COLUMN in 3.25 and this project supports API 27, whose
-            -- SQLite is older, so renaming means rebuilding the table behind a version bump —
-            -- and onDowngrade wipes the module configuration, so anyone who then flashed an
-            -- older build would lose theirs. That is a real cost for a name no user ever sees.
+            -- Dead since v5, read by nothing: `include new apps` is per user now and lives in
+            -- the table below, which this column cannot be -- one row per package is one answer
+            -- for every user at once. Kept because SQLite gained DROP COLUMN in 3.35 and this
+            -- supports API 27, so removing it means rebuilding the table.
             auto_include BOOLEAN DEFAULT 0 CHECK (auto_include IN (0, 1))
         );
         """
@@ -69,6 +67,21 @@ class Database(context: Context? = FakeContext()) :
         """
             .trimIndent())
 
+    db.execSQL(
+        """
+        CREATE TABLE IF NOT EXISTS auto_include (
+            -- One row per (module, user) that is auto-including; its existence is the whole
+            -- value, with no `enabled` column to disagree with it. Per user because a module two
+            -- profiles hold is two scopes, so it is two answers here too. Keyed by mid, so
+            -- uninstalling takes these rows with it exactly as it takes the module's scope.
+            mid integer NOT NULL,
+            user_id integer NOT NULL,
+            PRIMARY KEY (mid, user_id),
+            CONSTRAINT auto_include_module_constraint FOREIGN KEY (mid) REFERENCES modules (mid) ON DELETE CASCADE
+        );
+        """
+            .trimIndent())
+
     db.execSQL(
         """
         CREATE TABLE IF NOT EXISTS configs (
@@ -196,5 +209,30 @@ class Database(context: Context? = FakeContext()) :
             "ALTER TABLE modules ADD COLUMN auto_include BOOLEAN DEFAULT 0 CHECK (auto_include IN (0, 1));")
       }
     }
+    if (oldVersion < 5) {
+      db.execSQL(
+          """
+          CREATE TABLE IF NOT EXISTS auto_include (
+              mid integer NOT NULL,
+              user_id integer NOT NULL,
+              PRIMARY KEY (mid, user_id),
+              CONSTRAINT auto_include_module_constraint FOREIGN KEY (mid) REFERENCES modules (mid) ON DELETE CASCADE
+          );
+          """
+              .trimIndent())
+      // The old column meant every user in effect, so one row per user preserves it. The users
+      // come from `scope`, the only table naming any, unioned with user 0 -- not from
+      // `IUserManager`: a migration runs wherever the database is first opened, which is early,
+      // and one that silently found no users would read as a setting nobody ever made.
+      db.execSQL(
+          """
+          INSERT OR IGNORE INTO auto_include (mid, user_id)
+          SELECT m.mid, u.user_id
+          FROM modules m
+          JOIN (SELECT DISTINCT user_id FROM scope UNION SELECT 0 AS user_id) u
+          WHERE m.auto_include = 1;
+          """
+              .trimIndent())
+    }
   }
 }

daemon/src/main/kotlin/org/matrix/vector/daemon/data/ModuleDatabase.kt

@@ -63,35 +63,45 @@ object ModuleDatabase {
     return result
   }
 
-  fun getIncludeNewApps(packageName: String): Boolean {
+  /**
+   * Whether this copy of the module takes every newly installed app.
+   *
+   * Per user: a module two profiles hold is two copies with two scopes, and the column this
+   * replaces held one answer for the whole device.
+   */
+  fun getIncludeNewApps(packageName: String, userId: Int): Boolean {
     if (packageName == "lspd") return false
 
-    var isIncludeNewApps = false
-    dbHelper.readableDatabase
-        .query(
-            "modules",
-            arrayOf("auto_include"),
-            "module_pkg_name = ?",
-            arrayOf(packageName),
-            null,
-            null,
-            null)
-        .use { cursor ->
-          if (cursor.moveToFirst()) {
-            isIncludeNewApps = cursor.getInt(0) == 1
-          }
-        }
-    return isIncludeNewApps
+    return dbHelper.readableDatabase
+        .rawQuery(
+            """
+            SELECT 1 FROM auto_include a
+            JOIN modules m ON m.mid = a.mid
+            WHERE m.module_pkg_name = ? AND a.user_id = ?
+            """
+                .trimIndent(),
+            arrayOf(packageName, userId.toString()))
+        .use { it.moveToFirst() }
   }
 
-  fun modulesIncludingNewApps(): List<String> {
+  /**
+   * The modules auto-including in [userId], the user an app has just been installed into. A module
+   * wanting new apps in a work profile is not asking for the owner's.
+   */
+  fun modulesIncludingNewApps(userId: Int): List<String> {
     val result = mutableListOf<String>()
     dbHelper.readableDatabase
-        .query("modules", arrayOf("module_pkg_name"), "auto_include = 1", null, null, null, null)
+        .rawQuery(
+            """
+            SELECT m.module_pkg_name FROM auto_include a
+            JOIN modules m ON m.mid = a.mid
+            WHERE a.user_id = ?
+            """
+                .trimIndent(),
+            arrayOf(userId.toString()))
         .use { cursor ->
-          val idx = cursor.getColumnIndexOrThrow("module_pkg_name")
           while (cursor.moveToNext()) {
-            val pkgName = cursor.getString(idx)
+            val pkgName = cursor.getString(0)
             if (pkgName != "lspd") result.add(pkgName)
           }
         }
@@ -409,20 +419,50 @@ object ModuleDatabase {
     return res
   }
 
-  fun setIncludeNewApps(packageName: String, enabled: Boolean): Boolean {
+  /**
+   * Sets it for one user.
+   *
+   * Refuses a package no module row exists for, as the `UPDATE` this replaces did by matching
+   * nothing. `INSERT OR REPLACE` rather than `OR IGNORE`, so writing a value the table already
+   * holds still answers true, which the AIDL promises.
+   */
+  fun setIncludeNewApps(packageName: String, userId: Int, enabled: Boolean): Boolean {
     if (packageName == "lspd") return false
 
-    val values = ContentValues().apply { put("auto_include", if (enabled) 1 else 0) }
-
-    val changed =
-        dbHelper.writableDatabase.update(
-            "modules", values, "module_pkg_name = ?", arrayOf(packageName)) > 0
+    val db = dbHelper.writableDatabase
+    val mid = moduleIdOf(db, packageName) ?: return false
 
-    // If the auto_include flag changes, we should rebuild the scope cache
-    if (changed) {
-      ConfigCache.requestCacheUpdate()
+    if (enabled) {
+      val values =
+          ContentValues().apply {
+            put("mid", mid)
+            put("user_id", userId)
+          }
+      if (db.insertWithOnConflict(
+          "auto_include", null, values, SQLiteDatabase.CONFLICT_REPLACE) == -1L) {
+        Log.e(TAG, "Failed to store include-new-apps for $packageName in user $userId")
+        return false
+      }
+    } else {
+      db.delete("auto_include", "mid = ? AND user_id = ?", arrayOf(mid.toString(), userId.toString()))
     }
 
-    return changed
+    // As the column this replaces did. The switch changes no scope by itself, so dropping this
+    // is a separate question from whether the flag is per user.
+    ConfigCache.requestCacheUpdate()
+    return true
   }
+
+  /**
+   * The `mid` of a module row, or null. `simpleQueryForLong` throws on no row, so a caller that
+   * must tell an unknown package from a known one cannot use it bare.
+   */
+  private fun moduleIdOf(db: SQLiteDatabase, packageName: String): Long? =
+      runCatching {
+            db.compileStatement("SELECT mid FROM modules WHERE module_pkg_name = ?").use {
+              it.bindString(1, packageName)
+              it.simpleQueryForLong()
+            }
+          }
+          .getOrNull()
 }

daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ManagerService.kt

@@ -61,6 +61,12 @@ object ManagerService : IManagerService.Stub() {
    */
   private const val UNINSTALL_TIMEOUT_SECONDS = 60L
 
+  /**
+   * `PackageManager.INSTALL_SUCCEEDED`, which is hidden. Written out rather than tested as `>= 0`,
+   * so a future code that is neither success nor a negative `INSTALL_FAILED_*` reads as refusal.
+   */
+  private const val INSTALL_SUCCEEDED = 1
+
 
   private var managerPid = -1
   private var pendingManager = false
@@ -314,6 +320,34 @@ object ManagerService : IManagerService.Stub() {
     activityManager?.forceStopPackage(packageName, userId)
   }
 
+  /**
+   * Puts a package an existing user already holds into another user.
+   *
+   * Android 10 grew a fifth parameter, the permissions to allowlist, and this runs on 8.1 too.
+   * `null` there is what `pm install-existing` passes: allowlist nothing extra, not grant nothing.
+   */
+  override fun installExistingPackageAsUser(packageName: String, userId: Int): Boolean {
+    val pm = packageManager ?: return false
+    return runCatching {
+          val status =
+              if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+                pm.installExistingPackageAsUser(
+                    packageName, userId, 0, PackageManager.INSTALL_REASON_USER, null)
+              } else {
+                pm.installExistingPackageAsUser(
+                    packageName, userId, 0, PackageManager.INSTALL_REASON_USER)
+              }
+          // Reporting an unnamed code as success would leave the manager claiming a module is
+          // in a profile that never received it.
+          if (status != INSTALL_SUCCEEDED) {
+            Log.w(TAG, "install-existing of $packageName for user $userId answered $status")
+          }
+          status == INSTALL_SUCCEEDED
+        }
+        .onFailure { Log.e(TAG, "Failed to install $packageName for user $userId", it) }
+        .getOrDefault(false)
+  }
+
   override fun softReboot() = VectorDaemon.softReboot()
 
   /**
@@ -422,6 +456,37 @@ object ManagerService : IManagerService.Stub() {
     } ?: emptyList()
   }
 
+  /**
+   * The users holding [packageName], by the same `isPackageAvailable` predicate
+   * [getInstalledPackagesFromAllUsers] filters each user's rows by, so both mean one thing by
+   * "holds". One transaction per user: enumerating every package of every user to answer a
+   * question about one package is what exhausts this process's binder heap.
+   *
+   * Empty both for "no user holds it" and for a user service out of reach, which is why callers
+   * subtract it from [getUsers] rather than inverting it.
+   */
+  override fun getPackageUsers(packageName: String): IntArray {
+    val pm = packageManager ?: return IntArray(0)
+    val users = userManager?.getRealUsers() ?: return IntArray(0)
+    return users.filter { pm.isPackageAvailable(packageName, it.id, true) }.map { it.id }.toIntArray()
+  }
+
+  /**
+   * The user's photo, opened for the manager to read. Null for a user that has none, which is most.
+   *
+   * Reflectively: the `IUserManager` stub is a plain Java library with no Android SDK on its
+   * classpath, so it can name only the classes it declares and `ParcelFileDescriptor` is not one.
+   */
+  override fun getUserIcon(userId: Int): ParcelFileDescriptor? {
+    val manager = userManager ?: return null
+    return runCatching {
+          manager.javaClass
+              .getMethod("getUserIcon", Int::class.javaPrimitiveType)
+              .invoke(manager, userId) as? ParcelFileDescriptor
+        }
+        .getOrNull()
+  }
+
   override fun isSystemServerAttached() = SystemServerService.systemServerRequested
 
   override fun startActivityAsUser(intent: Intent, userId: Int, noUserSwitch: Boolean): Int {
@@ -522,10 +587,11 @@ object ManagerService : IManagerService.Stub() {
   override fun getDex2OatWrapperState() =
       if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) Dex2OatServer.compatibility else 0
 
-  override fun setIncludeNewApps(packageName: String, enabled: Boolean) =
-      ModuleDatabase.setIncludeNewApps(packageName, enabled)
+  override fun setIncludeNewApps(packageName: String, userId: Int, enabled: Boolean) =
+      ModuleDatabase.setIncludeNewApps(packageName, userId, enabled)
 
-  override fun getIncludeNewApps(packageName: String) = ModuleDatabase.getIncludeNewApps(packageName)
+  override fun getIncludeNewApps(packageName: String, userId: Int) =
+      ModuleDatabase.getIncludeNewApps(packageName, userId)
 
   override fun getRootImplementation() = RootImplementation.implementation
 

daemon/src/main/kotlin/org/matrix/vector/daemon/system/SystemExtensions.kt

@@ -197,34 +197,48 @@ private val getInstalledPackagesMethod: Method? by lazy {
       ?.apply { isAccessible = true }
 }
 
-/** Reflectively calls getInstalledPackages and casts to ParceledListSlice. */
+/**
+ * Reflectively calls getInstalledPackages and casts to ParceledListSlice.
+ *
+ * Null when the query did not happen, which an empty list would flatten into "this user holds
+ * nothing" — and the manager builds its whole app list out of this.
+ */
 private fun IPackageManager.getInstalledPackagesReflect(
     flags: Any,
     userId: Int
-): List<PackageInfo> {
-  val method = getInstalledPackagesMethod ?: return emptyList()
+): List<PackageInfo>? {
+  val method = getInstalledPackagesMethod ?: return null
   return runCatching {
         val result = method.invoke(this, flags, userId)
         @Suppress("UNCHECKED_CAST") (result as? ParceledListSlice<PackageInfo>)?.list
       }
       .onFailure { Log.e(TAG, "Reflection call failed", it.cause ?: it) }
-      .getOrNull() ?: emptyList()
+      .getOrNull()
 }
 
 fun IPackageManager.getInstalledPackagesFromAllUsers(
     flags: Int,
     filterNoProcess: Boolean
 ): List<PackageInfo> {
   val result = mutableListOf<PackageInfo>()
-  // Assuming userManager is available in this scope as in original code
-  val users = userManager?.getRealUsers() ?: emptyList()
+  // `getRealUsers` swallows a null binder and any failure into an empty list, and no device has
+  // no users, so an empty one is a question that could not be asked rather than an answer.
+  // `ConfigCache.rebuild` refuses it for the same reason.
+  val users = userManager?.getRealUsers().orEmpty()
+  if (users.isEmpty()) throw IllegalStateException("No users to enumerate packages for")
 
-  for (user in users) {
-    // We pass flags as Any so the reflective invoke handles Long or Int correctly
-    val flagParam: Any =
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) flags.toLong() else flags
+  // Any, so the reflective invoke boxes it as Long or Int to match.
+  val flagParam: Any =
+      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) flags.toLong() else flags
 
-    val infos = getInstalledPackagesReflect(flagParam, user.id)
+  for (user in users) {
+    // A user the platform will not answer for fails the whole call, as it did before the Kotlin
+    // rewrite: carrying on leaves a list short by a whole profile that nothing downstream can
+    // tell from the truth. Reachable on an ordinary device, where enumerating every package per
+    // user runs the binder buffer out and answers DeadObjectException.
+    val infos =
+        getInstalledPackagesReflect(flagParam, user.id)
+            ?: throw IllegalStateException("No package list for user ${user.id}")
     if (infos.isEmpty()) continue
 
     val validUserApps =