diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/Cli.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/Cli.kt index 8786ab82e..31386d245 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/Cli.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/Cli.kt @@ -276,7 +276,15 @@ class ModulesCommand { } } -@Command(name = "scope", description = ["Manage granular application injection scopes"]) +@Command( + name = "scope", + description = + [ + "Manage granular application injection scopes", + "A target is a package name, optionally `package/user_id`.", + "The system server is `system`; `android` is the framework package, whose", + "injectable process is android:ui.", + ]) class ScopeCommand { @ParentCommand lateinit var parent: Cli @@ -286,7 +294,7 @@ class ScopeCommand { return OutputFormatter.print(VectorIPC.transmit(req), parent.json) } - @Command(name = "add", description = ["Append apps to scope (format: pkg/user_id)"]) + @Command(name = "add", description = ["Append apps to scope (format: pkg/user_id, or `system`)"]) fun add( @Parameters(index = "0", paramLabel = "MODULE_PKG") modulePkg: String, @Parameters(index = "1..*") apps: List @@ -295,7 +303,9 @@ class ScopeCommand { return OutputFormatter.print(VectorIPC.transmit(req), parent.json) } - @Command(name = "set", description = ["Overwrite entire scope (format: pkg/user_id)"]) + @Command( + name = "set", + description = ["Overwrite entire scope (format: pkg/user_id, or `system`)"]) fun set( @Parameters(index = "0", paramLabel = "MODULE_PKG") modulePkg: String, @Parameters(index = "1..*") apps: List @@ -304,7 +314,7 @@ class ScopeCommand { return OutputFormatter.print(VectorIPC.transmit(req), parent.json) } - @Command(name = "rm", description = ["Remove apps from scope (format: pkg/user_id)"]) + @Command(name = "rm", description = ["Remove apps from scope (format: pkg/user_id, or `system`)"]) fun rm( @Parameters(index = "0", paramLabel = "MODULE_PKG") modulePkg: String, @Parameters(index = "1..*") apps: List diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt index aa22bc6a1..0a6f54d4e 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt @@ -495,12 +495,6 @@ object VectorService : IVectorDaemon.Stub() { when (action) { "approve" -> { - // "system" is the framework and not a package: it names system_server, which belongs - // to no package and resolves for nobody. The lookup below therefore came back null - // for every framework prompt, and the approval the user had just given was answered - // "Package not found" — the request was closed, its notification cancelled, and no - // row written. The normalisation to user 0 further down could never once have run. - // // Package by package rather than all-or-nothing: the prompt may have been up for an // hour and one of the packages it named can have been uninstalled in the meantime, // which is no reason to throw away the user's answer about the rest. @@ -511,7 +505,7 @@ object VectorService : IVectorDaemon.Stub() { // actually happened was that the user turned it down. val granted = scopePackageNames.filter { - it == "system" || packageManager?.getPackageInfoCompat(it, 0, userId) != null + ConfigCache.isScopeTargetAvailable(it, userId) == true } if (granted.isEmpty()) { // Logged, because until now this said nothing anywhere: the module was told diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt index b79aa1181..591917986 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/data/ConfigCache.kt @@ -42,6 +42,31 @@ object ConfigCache { /** The packages [modulePackage] claims, or null when it does not fix its scope. */ fun staticScopeOf(modulePackage: String): Set? = staticScopes[modulePackage] + /** + * Whether a scope row naming [packageName] under [userId] points at something that exists, or + * null when there is no package manager to ask. + * + * Three answers rather than two, for the reason the rebuild below refuses an empty user list: + * not being able to ask is not the same as being told no, and the CLI socket is listening before + * `main` has waited for the package service. A caller that acts on `false` must not act on + * `null`. + * + * `system` is the framework and not a package: it names the system server, which belongs to no + * package and resolves for nobody, so the package manager is the wrong thing to ask about it. + * Every other target is an ordinary package name, and a row naming one that is not installed + * reaches nothing — there is no process for the rebuild below to expand it into, and nothing + * anywhere said so. + * + * `isPackageAvailable` and not a plain `getPackageInfo`, because that is the predicate + * [getInstalledPackagesFromAllUsers] filters the manager's own list by: what the editor offers + * and what a write is allowed to name then mean one thing, hidden packages included. + */ + fun isScopeTargetAvailable(packageName: String, userId: Int): Boolean? { + if (packageName == "system") return true + val pm = packageManager ?: return null + return pm.isPackageAvailable(packageName, userId, true) + } + private val cacheUpdateChannel = Channel(Channel.CONFLATED) // Held for the whole of a rebuild, so that only one runs at a time. The channel serialises the @@ -376,6 +401,20 @@ object ConfigCache { val appUid = pkgInfo.applicationInfo!!.uid for (processName in processNames) { + // `system` at uid 1000 is the system server, and its module set is not read from here: + // `getModulesForSystemServer` asks the scope table for the rows spelled `system`, + // because it answers before any package manager exists to expand a package name with. + // So a row naming a package that runs a component in the system server expands to an + // entry nothing ever queries, and the module appeared to hold the framework while never + // being loaded into it. The entry goes; the row is still reported, so it can be removed. + if (processName == "system" && appUid == 1000) { + Log.w( + TAG, + "$appPkg runs a component in the system server, which is scoped as `system`; " + + "the row naming $appPkg cannot take $modPkg there") + continue + } + addToScope(processName, appUid, module) // A module in its own scope hooks itself in every user that has it — the copies share diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt index ffedb0e9f..1017629b8 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/CliHandler.kt @@ -104,11 +104,19 @@ object CliHandler { val modulePkg = request.targets[0] val apps = request.targets.drop(1) + // `pkg` or `pkg/user_id`, parsed once for every action: the three that take targets each + // spelled this out, and the check below has to read a target exactly as the write does. + fun target(appStr: String): ScopeEntry = + ScopeEntry().apply { + packageName = appStr.substringBefore('/') + userId = appStr.substringAfter('/', "").toIntOrNull() ?: 0 + } + // staticScope fixes the scope, so the CLI may still read it and take apps out of it, but not // put new ones in. Checked here as well as in the database so the answer names the packages. - fun rejectBeyondStaticScope(apps: List) { + fun rejectBeyondStaticScope(targets: List) { val claimed = ConfigCache.staticScopeOf(modulePkg) ?: return - val beyond = apps.map { it.substringBefore('/') }.filterNot { claimed.contains(it) } + val beyond = targets.map { it.packageName }.filterNot { claimed.contains(it) } if (beyond.isNotEmpty()) { throw IllegalArgumentException( "$modulePkg fixes its scope in module.prop, so ${beyond.joinToString()} cannot be " + @@ -116,6 +124,26 @@ object CliHandler { } } + // A target the daemon cannot reach is refused rather than written. Nothing checked, and the + // scope table takes any string: a name that resolves for nobody was accepted, listed back by + // `scope ls`, and reached nothing, which is a module that is configured and never loads with + // no error anywhere to say why. `scope rm` deliberately does not check, since a row already + // written this way has to stay removable. + // + // Refused on `false` alone and never on a null: this socket answers before the daemon has + // waited for the package service, and in that window nothing can be told apart from nothing. + fun rejectUnreachable(targets: List) { + val unknown = + targets + .filter { ConfigCache.isScopeTargetAvailable(it.packageName, it.userId) == false } + .map { "${it.packageName}/${it.userId}" } + if (unknown.isNotEmpty()) { + throw IllegalArgumentException( + "Not installed: ${unknown.joinToString()}. The system server is scoped as `system`; " + + "every other target is a package name, optionally `package/user_id`.") + } + } + return when (request.action) { "ls" -> { val scope = @@ -125,19 +153,14 @@ object CliHandler { } "add" -> { if (apps.isEmpty()) throw IllegalArgumentException("No target apps provided.") - rejectBeyondStaticScope(apps) + val targets = apps.map(::target) + rejectBeyondStaticScope(targets) + rejectUnreachable(targets) val scope = ModuleDatabase.getModuleScope(modulePkg) ?: mutableListOf() - apps.forEach { appStr -> - val parts = appStr.split("/") - val pkg = parts[0] - val user = parts.getOrNull(1)?.toIntOrNull() ?: 0 - if (scope.none { it.packageName == pkg && it.userId == user }) { - scope.add( - ScopeEntry().apply { - packageName = pkg - userId = user - }) + targets.forEach { entry -> + if (scope.none { it.packageName == entry.packageName && it.userId == entry.userId }) { + scope.add(entry) } } ModuleDatabase.setModuleScope(modulePkg, scope) @@ -146,29 +169,19 @@ object CliHandler { "set" -> { if (apps.isEmpty()) throw IllegalArgumentException("No target apps provided for scope overwrite.") - rejectBeyondStaticScope(apps) - val scope = mutableListOf() - apps.forEach { appStr -> - val parts = appStr.split("/") - val pkg = parts[0] - val user = parts.getOrNull(1)?.toIntOrNull() ?: 0 - scope.add( - ScopeEntry().apply { - packageName = pkg - userId = user - }) - } - ModuleDatabase.setModuleScope(modulePkg, scope) + val targets = apps.map(::target) + rejectBeyondStaticScope(targets) + rejectUnreachable(targets) + ModuleDatabase.setModuleScope(modulePkg, targets.toMutableList()) "Successfully overwrote scope for $modulePkg (${apps.size} apps)." } "rm" -> { if (apps.isEmpty()) throw IllegalArgumentException("No target apps provided to remove.") var removedCount = 0 - apps.forEach { appStr -> - val parts = appStr.split("/") - val pkg = parts[0] - val user = parts.getOrNull(1)?.toIntOrNull() ?: 0 - if (ModuleDatabase.removeModuleScope(modulePkg, pkg, user)) removedCount++ + apps.map(::target).forEach { entry -> + if (ModuleDatabase.removeModuleScope(modulePkg, entry.packageName, entry.userId)) { + removedCount++ + } } "Successfully removed $removedCount apps from $modulePkg scope." }