Give the manager ways in again, and name builds by where they came from (#809)

Flash Vector, open it from the root manager, close it, and there is no way back
in. The dialer code registered a filter with no action, which matches nothing, so
that branch has been dead since #597; it works again and is rebound to 832867,
VECTOR on the keypad. Parasitically the manager is not installed, so the launcher
has nothing to show either: the pinned shortcut and the standalone install that
#796 dropped return, in an "Opening Vector" section and a first-launch prompt,
with `getManagerApk()` handing over the APK the host cannot read. Sixteen new
strings, translated into all eighteen languages.

The version string now names where a build came from rather than calling every
canary "dirty", which it did only because the workflow writes signing credentials
into the tracked `gradle.properties`. Repository and commit both come from the
pull request's head, since GitHub's defaults describe the run and not the code.

Closes #815.

Author: JingMatrix

.github/workflows/core.yml

@@ -15,6 +15,14 @@ jobs:
     permissions:
       contents: write
     env:
+      # Where this build's code came from, for the version string the manager and module.prop show.
+      # Both are the head of the pull request rather than GitHub's defaults, which on a pull request
+      # describe the run instead of the code: GITHUB_REPOSITORY is this repository even when the
+      # branch came from a fork, and the checked-out HEAD is an ephemeral merge commit that exists
+      # nowhere and cannot be looked up by anyone who reads it off a device. Both head.* values are
+      # null outside a pull request, where the defaults are already right.
+      VECTOR_BUILD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
+      VECTOR_BUILD_COMMIT: ${{ github.event.pull_request.head.sha || github.sha }}
       CCACHE_COMPILERCHECK: "%compiler% -dumpmachine; %compiler% -dumpversion"
       CCACHE_NOHASHDIR: "true"
       CCACHE_HARDLINK: "true"

build.gradle.kts

@@ -4,6 +4,7 @@ import com.android.build.gradle.api.AndroidBasePlugin
 import com.ncorti.ktfmt.gradle.tasks.KtfmtFormatTask
 import java.io.ByteArrayOutputStream
 import javax.inject.Inject
+import org.gradle.api.provider.Property
 import org.gradle.api.provider.ValueSource
 import org.gradle.api.provider.ValueSourceParameters
 import org.gradle.process.ExecOperations
@@ -60,46 +61,142 @@ abstract class GitLatestTagValueSource : ValueSource<String, ValueSourceParamete
 }
 
 /**
- * The commit a build actually came from, short, with a marker for uncommitted changes.
+ * Which build this is, in one string: where it was built and from what commit.
  *
  * The version code is the commit count on origin/master, so every branch build carries master's
  * number: a build flashed from a feature branch and one flashed from master both report "v2.0
  * (3052)" and cannot be told apart on the device. That is not hypothetical — it cost a real
  * investigation to establish which of the two was installed.
  *
- * The hash goes in module.prop's human-readable version only. BuildConfig.VERSION_NAME, which is
- * what the manager prints on Home, is deliberately left clean.
+ * Two shapes, because "where did this binary come from" has two different answers:
+ *
+ * *On GitHub Actions*, `JingMatrix-Vector-93d66473` — the repository the code came from, then the
+ * commit. Forks build the same code from the same commits, so the hash alone identifies a
+ * *revision* and not a *build*, and a fork's artifact was indistinguishable from ours.
+ *
+ * Both halves are the *head* ones, and neither is read from the environment GitHub sets by default,
+ * because on a pull request both defaults name something other than the code that was built:
+ *
+ * `GITHUB_REPOSITORY` is the repository that *ran* the workflow. A pull request from a fork is built
+ * here, so it would stamp `JingMatrix/Vector` onto a branch that has never been near it — which is
+ * exactly the artifact the name most needs to distinguish.
+ *
+ * `HEAD` is worse. For a pull request the runner checks out an ephemeral merge of the branch into
+ * the base, so `git rev-parse HEAD` names a commit that exists in no repository, cannot be looked up
+ * by anyone who reads it off a device, and is gone once the run is. The commit that was pushed is
+ * the one that identifies the build.
+ *
+ * So the workflow passes both in: `VECTOR_BUILD_REPOSITORY` and `VECTOR_BUILD_COMMIT`, each taken
+ * from `github.event.pull_request.head.*` when there is a pull request and from the ordinary default
+ * when there is not — outside a pull request the two agree anyway. Absent either, this falls back to
+ * what the environment says and to `HEAD`, which covers a workflow too old to set them.
+ *
+ * *Locally*, the bare `93d66473`, or `93d66473-thinkpad` when the tree has uncommitted changes.
+ * That build corresponds to no commit at all, so naming the commit alone would name something the
+ * binary does not match — and whoever is looking at it is far better served by the machine it was
+ * built on than by the word "dirty", since a device that has a hand-built framework on it usually
+ * has exactly one candidate author.
+ *
+ * The dirty check is deliberately not applied on CI. The workflow's own "Write key" step appends
+ * the signing credentials to the tracked `gradle.properties` before Gradle starts, so every master
+ * and tag build reports a modified tree — every published canary has said `-dirty` for that reason
+ * and no other, which sent at least one user looking for a fault that was not there (#815).
+ *
+ * The result goes in module.prop's human-readable version and in BuildConfig.VERSION_HASH, which
+ * the manager shows on the status page. BuildConfig.VERSION_NAME is deliberately left clean.
  */
-abstract class GitCommitHashValueSource : ValueSource<String, ValueSourceParameters.None> {
+abstract class GitCommitHashValueSource : ValueSource<String, GitCommitHashValueSource.Parameters> {
+    interface Parameters : ValueSourceParameters {
+        /**
+         * `owner/repo` when GitHub Actions is building this, empty otherwise.
+         *
+         * Threaded in as parameters rather than read with `System.getenv` inside [obtain], which the
+         * configuration cache does not see and therefore does not invalidate on.
+         */
+        val buildRepository: Property<String>
+
+        /** The full SHA that was pushed, when CI knows one and it is not what is checked out. */
+        val buildCommit: Property<String>
+    }
+
     @get:Inject abstract val execOperations: ExecOperations
 
+    /**
+     * Runs [command] and returns its trimmed output, or null if it failed or said nothing.
+     *
+     * `exec` *throws* when the executable cannot be started at all, which `isIgnoreExitValue` does
+     * not cover — `hostname` is not on every machine — so the whole call is wrapped rather than just
+     * its exit code inspected.
+     */
+    private fun capture(vararg command: String): String? =
+        runCatching {
+                val output = ByteArrayOutputStream()
+                val result = execOperations.exec {
+                    commandLine(command.toList())
+                    standardOutput = output
+                    errorOutput = ByteArrayOutputStream()
+                    isIgnoreExitValue = true
+                }
+                if (result.exitValue != 0) null else output.toString().trim().ifBlank { null }
+            }
+            .getOrNull()
+
+    /**
+     * The machine's name, reduced to what is safe in a version string.
+     *
+     * A host name can carry anything the owner typed into it, and this value is interpolated into
+     * module.prop and a generated Kotlin string literal. Unknown hosts fall back to "local", which
+     * still reads correctly: not a CI build, and not a clean tree.
+     */
+    private fun hostname(): String {
+        val raw = capture("hostname") ?: System.getenv("HOSTNAME") ?: return "local"
+        // The short name only. A fully qualified host name is mostly domain, and the domain is the
+        // part that is identical across every machine that would ever build this.
+        val short = raw.substringBefore('.')
+        val safe = short.filter { it.isLetterOrDigit() || it == '-' || it == '_' }
+        return safe.ifBlank { "local" }
+    }
+
     override fun obtain(): String {
-        val hash = ByteArrayOutputStream()
-        val hashResult = execOperations.exec {
-            commandLine("git", "rev-parse", "--short", "HEAD")
-            standardOutput = hash
-            isIgnoreExitValue = true
-        }
-        if (hashResult.exitValue != 0 || hash.toString().isBlank()) return "unknown"
-
-        // A dirty tree is the case worth naming: that build corresponds to no commit at all, so
-        // "which commit is this" has no answer and the version should say so rather than name a
-        // commit the binary does not match.
-        val dirty = ByteArrayOutputStream()
-        val dirtyResult = execOperations.exec {
-            commandLine("git", "status", "--porcelain", "--untracked-files=no")
-            standardOutput = dirty
-            isIgnoreExitValue = true
+        // Abbreviated by git rather than by hand, so a CI build and a local build of the same commit
+        // read identically however long this repository's abbreviation happens to be.
+        val head = capture("git", "rev-parse", "--short", "HEAD") ?: return "unknown"
+
+        val repository = parameters.buildRepository.getOrElse("")
+        if (repository.isBlank()) {
+            val dirty = capture("git", "status", "--porcelain", "--untracked-files=no") != null
+            return if (dirty) "$head-${hostname()}" else head
         }
-        val suffix =
-            if (dirtyResult.exitValue == 0 && dirty.toString().isNotBlank()) "-dirty" else ""
-        return hash.toString().trim() + suffix
+
+        // The pushed commit is an ancestor of the merge that was checked out, so it is in the
+        // repository and git will abbreviate it. The truncation is only reached if it somehow is
+        // not, and a slightly odd length beats reporting the merge commit or nothing at all.
+        val pushed = parameters.buildCommit.getOrElse("").takeIf { it.isNotBlank() }
+        val short =
+            pushed?.let { capture("git", "rev-parse", "--short", it) ?: it.take(head.length) } ?: head
+        return repository.replace('/', '-') + "-" + short
     }
 }
 
 // This defers the execution of the git commands and allows Gradle to cache the results.
 val versionCodeProvider by extra(providers.of(GitCommitCountValueSource::class.java) {})
-val versionHashProvider by extra(providers.of(GitCommitHashValueSource::class.java) {})
+val versionHashProvider by
+    extra(
+        providers.of(GitCommitHashValueSource::class.java) {
+            // Set on every GitHub Actions runner and on nothing else, so the presence of either is
+            // the test for "this is a CI build". The workflow's own variables win because they name
+            // the branch that was pushed; GitHub's defaults name the run that built it.
+            parameters.buildRepository.set(
+                providers
+                    .environmentVariable("VECTOR_BUILD_REPOSITORY")
+                    .orElse(providers.environmentVariable("GITHUB_REPOSITORY"))
+                    .orElse("")
+            )
+            parameters.buildCommit.set(
+                providers.environmentVariable("VECTOR_BUILD_COMMIT").orElse("")
+            )
+        }
+    )
 val versionNameProvider by extra(providers.of(GitLatestTagValueSource::class.java) {})
 
 val injectedPackageName by extra("com.android.shell")

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

@@ -38,6 +38,9 @@ object VectorService : IDaemonService.Stub() {
       if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) TelephonyManager.ACTION_SECRET_CODE
       else Telephony.Sms.Intents.SECRET_CODE_ACTION
 
+  /** Dial *#*#832867#*#* ("VECTOR" on the keypad) to open the manager. */
+  private const val SECRET_CODE = "832867"
+
   override fun dispatchSystemServerContext(
       appThread: IBinder?,
       activityToken: IBinder?,
@@ -148,9 +151,9 @@ object VectorService : IDaemonService.Stub() {
         IntentFilter(NotificationManager.moduleScopeAction).apply { addDataScheme("module") }
 
     val secretCodeFilter =
-        IntentFilter().apply {
+        IntentFilter(ACTION_SECRET_CODE).apply {
           addDataScheme("android_secret_code")
-          addDataAuthority("5776733", null)
+          addDataAuthority(SECRET_CODE, null)
         }
 
     // Define strict Android 14+ flags and the system-only BRICK permission

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

@@ -37,6 +37,7 @@ import org.matrix.vector.daemon.data.PreferenceStore
 import org.matrix.vector.daemon.env.Dex2OatServer
 import org.matrix.vector.daemon.env.LogcatMonitor
 import org.matrix.vector.daemon.system.*
+import org.matrix.vector.daemon.utils.InstallerVerifier
 import org.matrix.vector.daemon.utils.PackageOptimizer
 import org.matrix.vector.daemon.utils.RootImplementation
 import org.matrix.vector.daemon.utils.applyXspaceWorkaround
@@ -293,6 +294,22 @@ object ManagerService : ILSPManagerService.Stub() {
 
   override fun softReboot() = VectorDaemon.softReboot()
 
+  /**
+   * The flashed manager APK, verified, for the manager to install as an ordinary app.
+   *
+   * The same file and the same check as [ApplicationService.requestInjectedManagerBinder], which
+   * serves it to the host process for injection — one APK, one signature gate, whichever way it
+   * leaves the module directory.
+   */
+  override fun getManagerApk(): ParcelFileDescriptor? =
+      runCatching {
+            InstallerVerifier.verifyInstallerSignature(FileSystem.managerApkPath.toString())
+            ParcelFileDescriptor.open(
+                FileSystem.managerApkPath.toFile(), ParcelFileDescriptor.MODE_READ_ONLY)
+          }
+          .onFailure { Log.e(TAG, "Failed to open or verify manager APK", it) }
+          .getOrNull()
+
   override fun reboot() {
     powerManager?.reboot(false, null, false)
   }

manager/build.gradle.kts

@@ -51,7 +51,8 @@ android {
 
     defaultConfig {
         applicationId = defaultManagerPackageName
-        // The commit this manager was built from, short, with a marker when the tree was dirty.
+        // Which build this manager is: the repository on CI, the machine when built locally from
+        // a modified tree, and the short commit either way. See GitCommitHashValueSource.
         // The version code is `git rev-list --count origin/master`, so a branch build and the
         // official build of the same depth are indistinguishable by number — and the manager and
         // the daemon are flashed separately, so they can be different builds of the same number.

manager/src/debug/kotlin/org/matrix/vector/manager/demo/FakeManagerService.kt

@@ -216,6 +216,14 @@ class FakeManagerService(
     override fun getLogPart(verbose: Boolean, name: String?): ParcelFileDescriptor? =
         real?.getLogPart(verbose, name)
 
+    /**
+     * Delegated, so the offer to install the manager is live or dead exactly as it really is.
+     *
+     * Null when there is no daemon behind the demo, which is the same answer the real one gives
+     * when it cannot serve the APK — and the status page renders that case rather than crashing.
+     */
+    override fun getManagerApk(): ParcelFileDescriptor? = real?.managerApk
+
     override fun getXposedVersionName(): String? = real?.xposedVersionName
 
     override fun clearLogs(verbose: Boolean): Boolean = real?.clearLogs(verbose) ?: false