From c1ec6f216bc7f5e63d855031293885a8925503ee Mon Sep 17 00:00:00 2001 From: laur Date: Sun, 20 Apr 2025 17:30:46 +0200 Subject: [PATCH 1/3] chore: add validations to RootInstaller - add extra sanity check to mount script - bail if package is still mounted after unmount execution - RootInstaller: - install(): - ensure mount script installation exits w/ 0; - ensure mount script execution exits w/ 0; - uninstall(): - bail if package-to-be-unmounted is not mounted; indicates likely error on user part - ensure unmount script exits w/ 0; - related to https://github.com/ReVanced/revanced-cli/issues/362 --- api/android/revanced-library.api | 2 ++ api/jvm/revanced-library.api | 2 ++ .../command/AdbShellCommandRunner.kt | 8 +++++++- .../library/installation/command/RunResult.kt | 9 +++++++++ .../installation/installer/AdbInstaller.kt | 1 - .../installation/installer/Constants.kt | 14 ++++++++++---- .../installation/installer/Installer.kt | 1 - .../installation/installer/RootInstaller.kt | 18 ++++++++++-------- 8 files changed, 40 insertions(+), 15 deletions(-) diff --git a/api/android/revanced-library.api b/api/android/revanced-library.api index 3deedb0..091f421 100644 --- a/api/android/revanced-library.api +++ b/api/android/revanced-library.api @@ -94,6 +94,7 @@ public final class app/revanced/library/installation/command/LocalShellCommandRu } public abstract interface class app/revanced/library/installation/command/RunResult { + public abstract fun ensureSuccess ()V public abstract fun getError ()Ljava/lang/String; public abstract fun getExitCode ()I public abstract fun getOutput ()Ljava/lang/String; @@ -101,6 +102,7 @@ public abstract interface class app/revanced/library/installation/command/RunRes } public final class app/revanced/library/installation/command/RunResult$DefaultImpls { + public static fun ensureSuccess (Lapp/revanced/library/installation/command/RunResult;)V public static fun waitFor (Lapp/revanced/library/installation/command/RunResult;)V } diff --git a/api/jvm/revanced-library.api b/api/jvm/revanced-library.api index 81007f9..80a48f6 100644 --- a/api/jvm/revanced-library.api +++ b/api/jvm/revanced-library.api @@ -70,6 +70,7 @@ public final class app/revanced/library/installation/command/AdbShellCommandRunn } public abstract interface class app/revanced/library/installation/command/RunResult { + public abstract fun ensureSuccess ()V public abstract fun getError ()Ljava/lang/String; public abstract fun getExitCode ()I public abstract fun getOutput ()Ljava/lang/String; @@ -77,6 +78,7 @@ public abstract interface class app/revanced/library/installation/command/RunRes } public final class app/revanced/library/installation/command/RunResult$DefaultImpls { + public static fun ensureSuccess (Lapp/revanced/library/installation/command/RunResult;)V public static fun waitFor (Lapp/revanced/library/installation/command/RunResult;)V } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt b/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt index aec0aa7..526de5e 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt @@ -41,10 +41,14 @@ class AdbShellCommandRunner : ShellCommandRunner { override fun waitFor() { process.waitFor() } + + override fun ensureSuccess() { + if (exitCode != 0) throw ShellCmdFailure() + } } } - override fun hasRootPermission(): Boolean = invoke("whoami").exitCode == 0 + override fun hasRootPermission(): Boolean = invoke("exit").exitCode == 0 override fun write(content: InputStream, targetFilePath: String) = device.push(content, System.currentTimeMillis(), 644, RemoteFile(targetFilePath)) @@ -56,4 +60,6 @@ class AdbShellCommandRunner : ShellCommandRunner { * @param targetFilePath The target file path. */ override fun move(file: File, targetFilePath: String) = device.push(file, RemoteFile(targetFilePath)) + + internal class ShellCmdFailure internal constructor() : Exception("Shell command execution failure") } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt b/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt index c5b0183..14d8838 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt @@ -1,5 +1,7 @@ package app.revanced.library.installation.command +import app.revanced.library.installation.command.AdbShellCommandRunner.ShellCmdFailure + /** * The result of a command execution. */ @@ -23,4 +25,11 @@ interface RunResult { * Waits for the command to finish. */ fun waitFor() {} + + /** + * Verifies whether the command exits with code 0. + * + * @throws ShellCmdFailure if given [RunResult] exited unsuccessfully. + */ + fun ensureSuccess() {} } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbInstaller.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbInstaller.kt index c8e389c..a4c74ba 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbInstaller.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbInstaller.kt @@ -2,7 +2,6 @@ package app.revanced.library.installation.installer import app.revanced.library.installation.command.AdbShellCommandRunner import app.revanced.library.installation.installer.Constants.INSTALLED_APK_PATH -import app.revanced.library.installation.installer.Installer.Apk import se.vidstige.jadb.JadbException import se.vidstige.jadb.managers.Package import se.vidstige.jadb.managers.PackageManager diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt index 24d9da1..fd5e4ba 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt @@ -19,14 +19,14 @@ internal object Constants { const val CREATE_INSTALLATION_PATH = "$CREATE_DIR $MOUNT_PATH" const val MOUNT_APK = - "base_path=\"$MOUNTED_APK_PATH\" && " + + "base_path='$MOUNTED_APK_PATH' && " + "mv $TMP_FILE_PATH \$base_path && " + "chmod 644 \$base_path && " + "chown system:system \$base_path && " + "chcon u:object_r:apk_data_file:s0 \$base_path" const val UMOUNT = - "grep $PLACEHOLDER /proc/mounts | " + + "$MOUNT_GREP | " + "while read -r line; do echo \$line | " + "cut -d ' ' -f 2 | " + "sed 's/apk.*/apk/' | " + @@ -49,6 +49,11 @@ internal object Constants { # Unmount any existing installations to prevent multiple unnecessary mounts. $UMOUNT + + # sanity check to make sure pkg is no longer mounted: + if $MOUNT_GREP >/dev/null; then + exit 1 + fi base_path="$MOUNTED_APK_PATH" @@ -56,13 +61,14 @@ internal object Constants { # Use Magisk mirror, if possible. if command -v magisk &> /dev/null; then - MIRROR="${'$'}(magisk --path)/.magisk/mirror" + MIRROR="$(magisk --path)/.magisk/mirror" fi - mount -o bind ${'$'}MIRROR${'$'}base_path ${'$'}stock_path + mount -o bind ${'$'}MIRROR${'$'}base_path ${'$'}stock_path || exit 1 # Kill the app to force it to restart the mounted APK in case it's currently running. $KILL + exit 0 """.trimIndent() /** diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/Installer.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/Installer.kt index da709ea..9add698 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/Installer.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/Installer.kt @@ -1,6 +1,5 @@ package app.revanced.library.installation.installer -import app.revanced.library.installation.installer.Installer.Apk import java.io.File import java.util.logging.Logger diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/RootInstaller.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/RootInstaller.kt index 2a3514a..5bc0cf9 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/RootInstaller.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/RootInstaller.kt @@ -16,8 +16,6 @@ import app.revanced.library.installation.installer.Constants.RESTART import app.revanced.library.installation.installer.Constants.TMP_FILE_PATH import app.revanced.library.installation.installer.Constants.UMOUNT import app.revanced.library.installation.installer.Constants.invoke -import app.revanced.library.installation.installer.Installer.Apk -import app.revanced.library.installation.installer.RootInstaller.NoRootPermissionException import java.io.File /** @@ -50,9 +48,8 @@ abstract class RootInstaller internal constructor( * @throws PackageNameRequiredException If the [Apk] does not have a package name. */ override suspend fun install(apk: Apk): RootInstallerResult { - logger.info("Installing ${apk.packageName} by mounting") - val packageName = apk.packageName?.also { it.assertInstalled() } ?: throw PackageNameRequiredException() + logger.info("Installing ${apk.packageName} by mounting") // Setup files. apk.file.move(TMP_FILE_PATH) @@ -61,8 +58,8 @@ abstract class RootInstaller internal constructor( // Install and run. TMP_FILE_PATH.write(MOUNT_SCRIPT(packageName)) - INSTALL_MOUNT_SCRIPT(packageName)().waitFor() - MOUNT_SCRIPT_PATH(packageName)().waitFor() + INSTALL_MOUNT_SCRIPT(packageName)().ensureSuccess() + MOUNT_SCRIPT_PATH(packageName)().ensureSuccess() RESTART(packageName)() DELETE(TMP_FILE_PATH)() @@ -71,13 +68,17 @@ abstract class RootInstaller internal constructor( } override suspend fun uninstall(packageName: String): RootInstallerResult { + packageName.assertInstalled() + if (MOUNT_GREP(packageName)().exitCode != 0) { + throw NotMountedException() + } logger.info("Uninstalling $packageName by unmounting") - UMOUNT(packageName)() + UMOUNT(packageName)().ensureSuccess() DELETE(MOUNTED_APK_PATH)(packageName)() DELETE(MOUNT_SCRIPT_PATH)(packageName)() - DELETE(TMP_FILE_PATH)() // Remove residual. + DELETE(TMP_FILE_PATH)() KILL(packageName)() @@ -131,4 +132,5 @@ abstract class RootInstaller internal constructor( internal class PackageNameRequiredException internal constructor() : Exception("Package name is required") internal class NoRootPermissionException internal constructor() : Exception("No root permission") + internal class NotMountedException internal constructor() : Exception("Package not mounted") } From 3cd3adea7698a055161fcfdd3f4effce949fea4a Mon Sep 17 00:00:00 2001 From: laur Date: Mon, 21 Apr 2025 00:40:38 +0200 Subject: [PATCH 2/3] [WIP] address review --- .gitignore | 2 +- .../library/installation/command/AdbShellCommandRunner.kt | 4 +--- .../app/revanced/library/installation/command/RunResult.kt | 4 ++-- .../library/installation/command/ShellCommandRunner.kt | 2 ++ .../library/installation/installer/AdbRootInstaller.kt | 2 -- .../app/revanced/library/installation/installer/Constants.kt | 2 +- 6 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 83f36ae..6732d12 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,4 @@ captures .cxx local.properties xcuserdata -node_modules/ \ No newline at end of file +node_modules/ diff --git a/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt b/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt index 526de5e..a264095 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/command/AdbShellCommandRunner.kt @@ -43,7 +43,7 @@ class AdbShellCommandRunner : ShellCommandRunner { } override fun ensureSuccess() { - if (exitCode != 0) throw ShellCmdFailure() + if (exitCode != 0) throw ShellCommandRunnerException() } } } @@ -60,6 +60,4 @@ class AdbShellCommandRunner : ShellCommandRunner { * @param targetFilePath The target file path. */ override fun move(file: File, targetFilePath: String) = device.push(file, RemoteFile(targetFilePath)) - - internal class ShellCmdFailure internal constructor() : Exception("Shell command execution failure") } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt b/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt index 14d8838..f54498f 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/command/RunResult.kt @@ -1,6 +1,6 @@ package app.revanced.library.installation.command -import app.revanced.library.installation.command.AdbShellCommandRunner.ShellCmdFailure +import app.revanced.library.installation.command.ShellCommandRunner.ShellCommandRunnerException /** * The result of a command execution. @@ -29,7 +29,7 @@ interface RunResult { /** * Verifies whether the command exits with code 0. * - * @throws ShellCmdFailure if given [RunResult] exited unsuccessfully. + * @throws ShellCommandRunnerException if given [RunResult] exited unsuccessfully. */ fun ensureSuccess() {} } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/command/ShellCommandRunner.kt b/src/commonMain/kotlin/app/revanced/library/installation/command/ShellCommandRunner.kt index 554ac0d..278edea 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/command/ShellCommandRunner.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/command/ShellCommandRunner.kt @@ -56,4 +56,6 @@ abstract class ShellCommandRunner internal constructor() { internal operator fun invoke( command: String, ) = runCommand("su -c \'$command\'") + + internal class ShellCommandRunnerException internal constructor() : Exception("Shell command execution failed") } diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbRootInstaller.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbRootInstaller.kt index d2d771f..66f5d0c 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbRootInstaller.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/AdbRootInstaller.kt @@ -1,8 +1,6 @@ package app.revanced.library.installation.installer import app.revanced.library.installation.command.AdbShellCommandRunner -import app.revanced.library.installation.installer.Installer.Apk -import app.revanced.library.installation.installer.RootInstaller.NoRootPermissionException /** * [AdbRootInstaller] for installing and uninstalling [Apk] files with using ADB root permissions by mounting. diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt index fd5e4ba..32bd3b7 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt @@ -50,7 +50,7 @@ internal object Constants { # Unmount any existing installations to prevent multiple unnecessary mounts. $UMOUNT - # sanity check to make sure pkg is no longer mounted: + # Sanity check to make sure pkg is no longer mounted. if $MOUNT_GREP >/dev/null; then exit 1 fi From 62b99e91f405168d0bfef5c2fb5531b6627215db Mon Sep 17 00:00:00 2001 From: laur Date: Wed, 23 Apr 2025 13:48:03 +0200 Subject: [PATCH 3/3] [WIP] address review --- .../revanced/library/installation/installer/Constants.kt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt index 32bd3b7..79c0c3e 100644 --- a/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt +++ b/src/commonMain/kotlin/app/revanced/library/installation/installer/Constants.kt @@ -49,11 +49,6 @@ internal object Constants { # Unmount any existing installations to prevent multiple unnecessary mounts. $UMOUNT - - # Sanity check to make sure pkg is no longer mounted. - if $MOUNT_GREP >/dev/null; then - exit 1 - fi base_path="$MOUNTED_APK_PATH" @@ -64,11 +59,10 @@ internal object Constants { MIRROR="$(magisk --path)/.magisk/mirror" fi - mount -o bind ${'$'}MIRROR${'$'}base_path ${'$'}stock_path || exit 1 + mount -o bind ${'$'}MIRROR${'$'}base_path ${'$'}stock_path # Kill the app to force it to restart the mounted APK in case it's currently running. $KILL - exit 0 """.trimIndent() /**