From 522669f2994a61a938bc89fcb4dc5a6afaef70ad Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Thu, 5 Nov 2026 22:16:42 +0100 Subject: [PATCH] Refuse a scope target that reaches nothing, and name the framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scope table takes any string, and nothing on the way in asked whether that string named anything. `handleScope` parsed a target and wrote it: a name that resolves for nobody was accepted, listed back by `scope ls`, and expanded to no process at all — a module that is configured and never loads, with nothing anywhere to say why. `scope add` and `scope set` now refuse such a target by name, through the same predicate the manager's own installed list is filtered by, so what the editor offers and what a write may name mean one thing. The predicate has a third answer for having no package manager to ask, which this socket can reach before `main` has waited for one and which must not be read as a refusal, and `scope rm` still takes anything, because a row already written this way has to stay removable. `system` is the system server and `android` is the ordinary framework package, and the CLI spelled out neither. A row naming `android` is a legitimate row — that package's injectable process is android:ui — but it cannot reach the system server: `getModulesForSystemServer` asks the scope table for the rows spelled `system` alone, because it answers before any package manager exists to expand a package name with. The rebuild nevertheless expanded such a row into a `system`/1000 entry, which nothing ever queries, so the module held the framework in the configuration and was never loaded into it. That entry goes, and the rebuild now names the package it came from. The three target-taking commands and `scope --help` say which word means which. Also here: the scope request path kept its own copy of the "is this target there" test, which is now the one predicate beside the rebuild that depends on it, and the three commands share one target parse rather than spelling `pkg/user_id` out apiece. Fixes #1061 --- .../kotlin/org/matrix/vector/daemon/Cli.kt | 18 ++++- .../org/matrix/vector/daemon/VectorService.kt | 8 +- .../matrix/vector/daemon/data/ConfigCache.kt | 39 ++++++++++ .../matrix/vector/daemon/ipc/CliHandler.kt | 75 +++++++++++-------- 4 files changed, 98 insertions(+), 42 deletions(-) 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 8786ab82e3..31386d2454 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 aa22bc6a19..0a6f54d4ef 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 b79aa11817..591917986e 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 ffedb0e9f3..1017629b8b 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." }