Skip to content

Fix the scope requests that cannot be granted and the answers that are dropped #890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JingMatrix merged 1 commit into from
Aug 7, 2026
+168 −89
Merged

Fix the scope requests that cannot be granted and the answers that are dropped #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 91 additions & 42 deletions daemon/src/main/kotlin/org/matrix/vector/daemon/VectorService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,29 +385,39 @@ object VectorService : IVectorDaemon.Stub() {
val packageName = parts[0]
val userId = parts[1].toIntOrNull() ?: return

val scopePackageName = data.path?.substring(1) ?: return // remove leading '/'
// Everything the one prompt asked about, in the order it listed them. ',' cannot occur in a
// package name, so this is the list the module named and not a guess at it.
val scopePackageNames =
data.path?.substring(1)?.split(",")?.filter { it.isNotEmpty() } ?: return // strip '/'
if (scopePackageNames.isEmpty()) return
val action = data.getQueryParameter("action") ?: return

// A prompt outlives the process that asked for it: it sits for an hour, and the app the module
// is running inside can be killed at any point in that hour. For approve, deny and the timeout
// there is then nobody left to tell, so those are dropped where they always were. "Never ask
// again" is not like them — it is the user's decision about the module, not an answer owed to a
// caller who is still listening — so it is honoured whether or not anyone is there to hear it,
// and the claim below still takes the prompt down.
if (!callbackBinder.isBinderAlive && action != "block") return

// One prompt reaches this receiver from four places — its three buttons and its delete intent
// — and a swipe or the one-hour timeout fires the delete intent whether or not a button was
// pressed first. Answering the module twice, an approval followed by a spurious "Request
// timeout", would be worse than the dismissal never reaching it, so whichever of the four
// arrives first is the one that answers and the rest are dropped. The request is identified by
// the module, its user and the package it asked for; the action is deliberately not part of
// that, since the whole point is that a second *different* action must not answer again.
if (!NotificationManager.claimScopeAnswer(packageName, userId, scopePackageName)) {
Log.d(TAG, "Ignoring $action of $scopePackageName for $packageName: already answered")
// the module, its user and the set of packages it asked for; the action is deliberately not
// part of that, since the whole point is that a second *different* action must not answer
// again.
if (!NotificationManager.claimScopeAnswer(packageName, userId, scopePackageNames)) {
Log.d(
TAG,
"Ignoring $action of ${scopePackageNames.joinToString()} for $packageName:" +
" already answered")
return
}

// A prompt outlives the process that asked for it: it sits for an hour, and the app the module
// is running inside can be killed at any point in that hour. Nothing is dropped for that any
// more. There used to be a `!callbackBinder.isBinderAlive` return above the claim, exempting
// only "never ask again", and it did more than lose an answer nobody was listening for: an
// approval the user had already given was thrown away, and because the return sat above the
// claim and the cancel, the prompt stayed on screen with live buttons that did nothing for the
// rest of the hour. An approval is a decision about the module's scope and is written down
// whether or not the module is there to hear it; deny and the timeout have nothing to record
// but still have a prompt to take down. What actually fails against a dead module is the
// callback below, and that is caught where it is made.
val iCallback = IXposedScopeCallback.Stub.asInterface(callbackBinder)
runCatching {
// Answered before the requested package is looked up at all, because "never ask again" is
Expand All @@ -419,12 +429,10 @@ object VectorService : IVectorDaemon.Stub() {
// down, and the module was free to ask again a second later.
if (action == "block") {
blockScopeRequests(packageName)
// The preference only stops the *next* request. A module that asked for three packages
// has a prompt up for each, so without this the user says "never ask again" and is left
// The preference only stops the *next* request. A module that asked three times has a
// prompt up for each, so without this the user says "never ask again" and is left
// looking at two more questions, both still approvable. Each withdrawn request is
// answered in its own right, because each of them was asked in its own right; what that
// costs a module whose listener expects one call is written out on
// withdrawScopeRequests.
// answered in its own right, because each of them was asked in its own right.
NotificationManager.withdrawScopeRequests(packageName).forEach { pending ->
runCatching { pending.onScopeRequestFailed("Request blocked by configuration") }
}
Expand All @@ -437,31 +445,72 @@ object VectorService : IVectorDaemon.Stub() {
return@runCatching
}

val appInfo = packageManager?.getPackageInfoCompat(scopePackageName, 0, userId)
if (appInfo == null) {
// Leaving the whole function here skipped the cancel below, which used to be merely
// untidy and is now a prompt nobody can use: the request has been answered, so every
// later press of its buttons is dropped. The request is over either way, so the
// notification goes with it.
iCallback.onScopeRequestFailed("Package not found")
return@runCatching
}
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.
//
// Only under "approve", because only an approval has to name something real. Deny
// and the timeout used to be refused here too, so dismissing a prompt for a package
// that had since been uninstalled told the module "Package not found" when what had
// actually happened was that the user turned it down.
val granted =
scopePackageNames.filter {
it == "system" || packageManager?.getPackageInfoCompat(it, 0, userId) != null
}
if (granted.isEmpty()) {
// Logged, because until now this said nothing anywhere: the module was told
// "Package not found", the user was told nothing at all, and the daemon kept no
// record that the press had even arrived. The framework-scope failure above was
// invisible for exactly that reason.
Log.w(
TAG,
"None of ${scopePackageNames.joinToString()} resolve for user $userId;" +
" refusing the scope request of $packageName")
// Leaving the whole function here skipped the cancel below, which used to be
// merely untidy and is now a prompt nobody can use: the request has been answered,
// so every later press of its buttons is dropped. The request is over either way,
// so the notification goes with it.
iCallback.onScopeRequestFailed("Package not found")
return@runCatching
}
val scopes = ModuleDatabase.getModuleScope(packageName) ?: mutableListOf()
// Compared against where the row will land, not against the user who asked: the
// framework is stored under user 0 whoever requested it, so for "system" this test
// never matched and every approval appended a duplicate and rewrote the whole table.
val storedUserId = if (scopePackageName == "system") 0 else userId
if (scopes.none { it.packageName == scopePackageName && it.userId == storedUserId }) {
scopes.add(
ScopeEntry().apply {
this.packageName = scopePackageName
this.userId = storedUserId
})
ModuleDatabase.setModuleScope(packageName, scopes)
var added = false
granted.forEach { scopePackageName ->
// Compared against where the row will land, not against the user who asked: the
// framework is stored under user 0 whoever requested it, so for "system" this test
// never matched and every approval appended a duplicate and rewrote the whole
// table.
val storedUserId = if (scopePackageName == "system") 0 else userId
val present =
scopes.any { it.packageName == scopePackageName && it.userId == storedUserId }
if (!present) {
scopes.add(
ScopeEntry().apply {
this.packageName = scopePackageName
this.userId = storedUserId
})
added = true
}
}
iCallback.onScopeRequestApproved(listOf(scopePackageName))
// One write for the whole prompt, and none at all when the user approved what the
// module already had. `setModuleScope` replaces the module's rows wholesale and
// enables the module on the way through, so writing per package would rewrite the
// table once per package — leaving a window after each in which the scope is only
// partly what was agreed to — and writing unconditionally would let a module enable
// itself by asking again for what it has.
if (added) ModuleDatabase.setModuleScope(packageName, scopes)
Log.i(TAG, "Approved ${granted.joinToString()} for $packageName on user $userId")
// The packages that were granted, which is what the list in this callback is for. A
// module comparing it against what it asked for can see what it did not get.
iCallback.onScopeRequestApproved(granted)
}
"deny" -> iCallback.onScopeRequestFailed("Request denied by user")
"delete" -> iCallback.onScopeRequestFailed("Request timeout")
Expand All @@ -470,9 +519,9 @@ object VectorService : IVectorDaemon.Stub() {
// onScopeRequestFailed declares @NonNull, and Throwable.message is frequently null.
.onFailure { runCatching { iCallback.onScopeRequestFailed(it.message ?: it.toString()) } }

// Only this one request goes; a module that asked for several packages has a prompt still open
// for each of the others, and they are answered on their own.
NotificationManager.cancelScopeRequest(packageName, userId, scopePackageName)
// Only this one request goes; a module that asked more than once has a prompt still open for
// each of its other requests, and they are answered on their own.
NotificationManager.cancelScopeRequest(packageName, userId, scopePackageNames)
}

/**
Expand Down
25 changes: 20 additions & 5 deletions daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ModuleAppService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,26 @@ class ModuleAppService(private val loadedModule: LoadedModule) : IXposedService.
?.distinct() ?: emptyList()
}

/**
* One request, one question, one answer.
*
* The AIDL hands over a list and takes a single [IXposedScopeCallback] for it, and the javadoc on
* the client's `OnScopeEventListener` says its listener runs "when the request is completed" —
* singular — with `onScopeRequestApproved` taking the *packages* that were approved. This used to
* put one prompt per package on screen, each answered in its own right, so a module asking for
* three packages made the user answer three questions and then fired that one listener three
* times. A module that took the first answer as the answer acted on a third of it.
*
* So the whole list goes up as one prompt and Approve answers for all of it. What the user gives
* away in one press is what the prompt lists, which is why it lists all of them rather than a
* count, and why the packages are sorted and deduplicated first: it is a set that is being agreed
* to, the same set asked for twice is the same question, and `NotificationManager` identifies a
* prompt by the set it names.
*/
override fun requestScope(packages: List<String>, callback: IXposedScopeCallback) {
val userId = ensureModule()
if (packages.isEmpty()) {
val requested = packages.distinct().sorted()
if (requested.isEmpty()) {
// Nothing was asked for, so the request is trivially satisfied. Returning without touching
// the callback would leave the module waiting forever.
callback.onScopeRequestApproved(emptyList())
Expand All @@ -205,17 +222,15 @@ class ModuleAppService(private val loadedModule: LoadedModule) : IXposedService.
// A module that fixed its own scope in module.prop does not get to ask for more of it at
// runtime. Prompting the user here would make "fixed" mean nothing.
ConfigCache.staticScopeOf(loadedModule.packageName)?.let { claimed ->
val beyond = packages.filterNot { claimed.contains(it) }
val beyond = requested.filterNot { claimed.contains(it) }
if (beyond.isNotEmpty()) {
callback.onScopeRequestFailed(
"This module declares a static scope, so ${beyond.joinToString()} cannot be added")
return
}
}
if (!PreferenceStore.isScopeRequestBlocked(loadedModule.packageName)) {
packages.forEach { pkg ->
NotificationManager.requestModuleScope(loadedModule.packageName, userId, pkg, callback)
}
NotificationManager.requestModuleScope(loadedModule.packageName, userId, requested, callback)
} else {
callback.onScopeRequestFailed("Scope request blocked by user configuration")
}
Expand Down
Loading
Loading