From 7a072e6d3b93be4398d8c6dc00d1d731aca04134 Mon Sep 17 00:00:00 2001 From: Daniel Alome Date: Thu, 1 Oct 2026 23:25:45 +0100 Subject: [PATCH 1/2] style: spotless reformat, no functional change --- .../manager/services/IdeCommandServiceImpl.kt | 466 +++++++++--------- 1 file changed, 239 insertions(+), 227 deletions(-) diff --git a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt index 3d8cbbb9ff..2beb041cbf 100644 --- a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt +++ b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt @@ -25,235 +25,247 @@ import java.util.UUID import java.util.concurrent.ConcurrentHashMap class IdeCommandServiceImpl( - private val pluginId: String, - private val permissions: Set, - private val projectRootProvider: () -> File?, - private val appFilesDir: File + private val pluginId: String, + private val permissions: Set, + private val projectRootProvider: () -> File?, + private val appFilesDir: File, ) : IdeCommandService { - - private val runningCommands = ConcurrentHashMap() - - override fun executeCommand(spec: CommandSpec, timeoutMs: Long): CommandExecution { - requirePermission() - requireConcurrencyLimit() - - val executionId = "$pluginId-${UUID.randomUUID()}" - val projectRoot = projectRootProvider() - - val processBuilder = when (spec) { - is CommandSpec.ShellCommand -> { - val workDir = when { - spec.workingDirectory == null -> projectRoot - Paths.get(spec.workingDirectory).isAbsolute -> File(spec.workingDirectory) - else -> projectRoot?.let { File(it, spec.workingDirectory).canonicalFile } - } - validateWorkingDirectory(workDir) - ProcessBuilder(listOf(spec.executable) + spec.arguments).apply { - workDir?.let { directory(it) } - environment().putAll(spec.environment) - } - } - is CommandSpec.GradleTask -> { - val gradleWrapper = projectRoot?.let { File(it, "gradlew") } - ?: throw IllegalStateException("No project root available for Gradle task execution") - if (!gradleWrapper.exists()) { - throw IllegalStateException("Gradle wrapper not found at ${gradleWrapper.absolutePath}") - } - if (!gradleWrapper.canExecute()) { - throw IllegalStateException("Gradle wrapper is not executable: ${gradleWrapper.absolutePath}") - } - ProcessBuilder(listOf(gradleWrapper.absolutePath, spec.taskPath) + spec.arguments).apply { - directory(projectRoot) - } - } - } - - processBuilder.redirectErrorStream(false) - injectTermuxEnvironment(processBuilder) - - val execution = CommandExecutionImpl( - executionId = executionId, - processBuilder = processBuilder, - timeoutMs = timeoutMs - ) - runningCommands[executionId] = execution - execution.start { runningCommands.remove(executionId) } - return execution - } - - override fun isCommandRunning(executionId: String): Boolean { - return runningCommands[executionId]?.isRunning() == true - } - - override fun cancelCommand(executionId: String): Boolean { - return runningCommands[executionId]?.let { - it.cancel() - true - } ?: false - } - - override fun getRunningCommandCount(): Int = runningCommands.size - - fun cancelAllCommands() { - runningCommands.entries.removeAll { (_, execution) -> - execution.cancel() - true - } - } - - private fun requirePermission() { - if (PluginPermission.SYSTEM_COMMANDS !in permissions) { - throw SecurityException( - "Plugin $pluginId does not have SYSTEM_COMMANDS permission" - ) - } - } - - private fun requireConcurrencyLimit() { - if (runningCommands.size >= MAX_CONCURRENT_COMMANDS) { - throw IllegalStateException( - "Plugin $pluginId has reached the maximum of $MAX_CONCURRENT_COMMANDS concurrent commands" - ) - } - } - - private fun validateWorkingDirectory(dir: File?) { - if (dir == null) return - val projectRoot = projectRootProvider() ?: return - val normalizedDir = dir.canonicalFile.toPath() - val normalizedRoot = projectRoot.canonicalFile.toPath() - if (normalizedDir != normalizedRoot && !normalizedDir.startsWith(normalizedRoot)) { - throw SecurityException( - "Plugin $pluginId attempted to execute in directory outside project root: $normalizedDir" - ) - } - } - - private fun injectTermuxEnvironment(processBuilder: ProcessBuilder) { - val termuxBase = appFilesDir.absolutePath - val termuxBin = "$termuxBase/usr/bin" - val termuxLib = "$termuxBase/usr/lib" - val env = processBuilder.environment() - - val existingPath = env["PATH"] ?: "" - if (!existingPath.contains(termuxBin)) { - env["PATH"] = "$termuxBin:$existingPath" - } - - val existingLdPath = env["LD_LIBRARY_PATH"] ?: "" - if (!existingLdPath.contains(termuxLib)) { - env["LD_LIBRARY_PATH"] = "$termuxLib:$existingLdPath" - } - - env.putIfAbsent("HOME", "$termuxBase/home") - env.putIfAbsent("TMPDIR", "$termuxBase/usr/tmp") - env.putIfAbsent("LANG", "en_US.UTF-8") - env.putIfAbsent("PREFIX", "$termuxBase/usr") - } - - companion object { - private const val MAX_CONCURRENT_COMMANDS = 3 - } + private val runningCommands = ConcurrentHashMap() + + override fun executeCommand( + spec: CommandSpec, + timeoutMs: Long, + ): CommandExecution { + requirePermission() + requireConcurrencyLimit() + + val executionId = "$pluginId-${UUID.randomUUID()}" + val projectRoot = projectRootProvider() + + val processBuilder = + when (spec) { + is CommandSpec.ShellCommand -> { + val workDir = + when { + spec.workingDirectory == null -> projectRoot + Paths.get(spec.workingDirectory).isAbsolute -> File(spec.workingDirectory) + else -> projectRoot?.let { File(it, spec.workingDirectory).canonicalFile } + } + validateWorkingDirectory(workDir) + ProcessBuilder(listOf(spec.executable) + spec.arguments).apply { + workDir?.let { directory(it) } + environment().putAll(spec.environment) + } + } + + is CommandSpec.GradleTask -> { + val gradleWrapper = + projectRoot?.let { File(it, "gradlew") } + ?: throw IllegalStateException("No project root available for Gradle task execution") + if (!gradleWrapper.exists()) { + throw IllegalStateException("Gradle wrapper not found at ${gradleWrapper.absolutePath}") + } + if (!gradleWrapper.canExecute()) { + throw IllegalStateException("Gradle wrapper is not executable: ${gradleWrapper.absolutePath}") + } + ProcessBuilder(listOf(gradleWrapper.absolutePath, spec.taskPath) + spec.arguments).apply { + directory(projectRoot) + } + } + } + + processBuilder.redirectErrorStream(false) + injectTermuxEnvironment(processBuilder) + + val execution = + CommandExecutionImpl( + executionId = executionId, + processBuilder = processBuilder, + timeoutMs = timeoutMs, + ) + runningCommands[executionId] = execution + execution.start { runningCommands.remove(executionId) } + return execution + } + + override fun isCommandRunning(executionId: String): Boolean = runningCommands[executionId]?.isRunning() == true + + override fun cancelCommand(executionId: String): Boolean = + runningCommands[executionId]?.let { + it.cancel() + true + } ?: false + + override fun getRunningCommandCount(): Int = runningCommands.size + + fun cancelAllCommands() { + runningCommands.entries.removeAll { (_, execution) -> + execution.cancel() + true + } + } + + private fun requirePermission() { + if (PluginPermission.SYSTEM_COMMANDS !in permissions) { + throw SecurityException( + "Plugin $pluginId does not have SYSTEM_COMMANDS permission", + ) + } + } + + private fun requireConcurrencyLimit() { + if (runningCommands.size >= MAX_CONCURRENT_COMMANDS) { + throw IllegalStateException( + "Plugin $pluginId has reached the maximum of $MAX_CONCURRENT_COMMANDS concurrent commands", + ) + } + } + + private fun validateWorkingDirectory(dir: File?) { + if (dir == null) return + val projectRoot = projectRootProvider() ?: return + val normalizedDir = dir.canonicalFile.toPath() + val normalizedRoot = projectRoot.canonicalFile.toPath() + if (normalizedDir != normalizedRoot && !normalizedDir.startsWith(normalizedRoot)) { + throw SecurityException( + "Plugin $pluginId attempted to execute in directory outside project root: $normalizedDir", + ) + } + } + + private fun injectTermuxEnvironment(processBuilder: ProcessBuilder) { + val termuxBase = appFilesDir.absolutePath + val termuxBin = "$termuxBase/usr/bin" + val termuxLib = "$termuxBase/usr/lib" + val env = processBuilder.environment() + + val existingPath = env["PATH"] ?: "" + if (!existingPath.contains(termuxBin)) { + env["PATH"] = "$termuxBin:$existingPath" + } + + val existingLdPath = env["LD_LIBRARY_PATH"] ?: "" + if (!existingLdPath.contains(termuxLib)) { + env["LD_LIBRARY_PATH"] = "$termuxLib:$existingLdPath" + } + + env.putIfAbsent("HOME", "$termuxBase/home") + env.putIfAbsent("TMPDIR", "$termuxBase/usr/tmp") + env.putIfAbsent("LANG", "en_US.UTF-8") + env.putIfAbsent("PREFIX", "$termuxBase/usr") + } + + companion object { + private const val MAX_CONCURRENT_COMMANDS = 3 + } } private class CommandExecutionImpl( - override val executionId: String, - private val processBuilder: ProcessBuilder, - private val timeoutMs: Long + override val executionId: String, + private val processBuilder: ProcessBuilder, + private val timeoutMs: Long, ) : CommandExecution { - - private val outputChannel = Channel(capacity = Channel.UNLIMITED) - private val resultDeferred = CompletableDeferred() - private val scope = CoroutineScope(Dispatchers.IO + Job()) - private var process: Process? = null - private val stdoutBuilder = StringBuilder() - private val stderrBuilder = StringBuilder() - - override val output: Flow = outputChannel.receiveAsFlow() - - fun start(onComplete: () -> Unit) { - scope.launch { - val startTime = System.currentTimeMillis() - - runCatching { - withTimeout(timeoutMs) { - process = processBuilder.start() - val proc = process!! - - val stdoutJob = launch { readStream(proc, isStdErr = false) } - val stderrJob = launch { readStream(proc, isStdErr = true) } - - val exitCode = proc.waitFor() - stdoutJob.join() - stderrJob.join() - - outputChannel.send(CommandOutput.ExitCode(exitCode)) - outputChannel.close() - - val duration = System.currentTimeMillis() - startTime - if (exitCode == 0) { - CommandResult.Success(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), duration) - } else { - CommandResult.Failure(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), null, duration) - } - } - }.onSuccess { result -> - resultDeferred.complete(result) - }.onFailure { e -> - process?.destroyForcibly() - outputChannel.close() - val stdout = stdoutBuilder.toString() - val stderr = stderrBuilder.toString() - val duration = System.currentTimeMillis() - startTime - val failureResult = when (e) { - is kotlinx.coroutines.TimeoutCancellationException -> - CommandResult.Failure(-1, stdout, stderr, "Command timed out after ${timeoutMs}ms: ${e.message}", duration) - is kotlinx.coroutines.CancellationException -> - CommandResult.Cancelled(stdout, stderr) - else -> - CommandResult.Failure(-1, stdout, stderr, "Unexpected error: ${e.message}", duration) - } - if (resultDeferred.isActive) { - resultDeferred.complete(failureResult) - } - } - - onComplete() - } - } - - private suspend fun readStream(process: Process, isStdErr: Boolean) { - val stream = if (isStdErr) process.errorStream else process.inputStream - val builder = if (isStdErr) stderrBuilder else stdoutBuilder - BufferedReader(InputStreamReader(stream)).use { reader -> - var line = reader.readLine() - while (line != null) { - if (builder.length + line.length <= MAX_OUTPUT_BYTES) { - builder.appendLine(line) - } - val output = if (isStdErr) CommandOutput.StdErr(line) else CommandOutput.StdOut(line) - outputChannel.send(output) - line = reader.readLine() - } - } - } - - override suspend fun await(): CommandResult = resultDeferred.await() - - override fun cancel() { - process?.destroyForcibly() - outputChannel.close() - if (resultDeferred.isActive) { - resultDeferred.complete( - CommandResult.Cancelled(stdoutBuilder.toString(), stderrBuilder.toString()) - ) - } - scope.cancel() - } - - fun isRunning(): Boolean = process?.isAlive == true - - companion object { - private const val MAX_OUTPUT_BYTES = 10 * 1024 * 1024 - } -} \ No newline at end of file + private val outputChannel = Channel(capacity = Channel.UNLIMITED) + private val resultDeferred = CompletableDeferred() + private val scope = CoroutineScope(Dispatchers.IO + Job()) + private var process: Process? = null + private val stdoutBuilder = StringBuilder() + private val stderrBuilder = StringBuilder() + + override val output: Flow = outputChannel.receiveAsFlow() + + fun start(onComplete: () -> Unit) { + scope.launch { + val startTime = System.currentTimeMillis() + + runCatching { + withTimeout(timeoutMs) { + process = processBuilder.start() + val proc = process!! + + val stdoutJob = launch { readStream(proc, isStdErr = false) } + val stderrJob = launch { readStream(proc, isStdErr = true) } + + val exitCode = proc.waitFor() + stdoutJob.join() + stderrJob.join() + + outputChannel.send(CommandOutput.ExitCode(exitCode)) + outputChannel.close() + + val duration = System.currentTimeMillis() - startTime + if (exitCode == 0) { + CommandResult.Success(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), duration) + } else { + CommandResult.Failure(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), null, duration) + } + } + }.onSuccess { result -> + resultDeferred.complete(result) + }.onFailure { e -> + process?.destroyForcibly() + outputChannel.close() + val stdout = stdoutBuilder.toString() + val stderr = stderrBuilder.toString() + val duration = System.currentTimeMillis() - startTime + val failureResult = + when (e) { + is kotlinx.coroutines.TimeoutCancellationException -> { + CommandResult.Failure(-1, stdout, stderr, "Command timed out after ${timeoutMs}ms: ${e.message}", duration) + } + + is kotlinx.coroutines.CancellationException -> { + CommandResult.Cancelled(stdout, stderr) + } + + else -> { + CommandResult.Failure(-1, stdout, stderr, "Unexpected error: ${e.message}", duration) + } + } + if (resultDeferred.isActive) { + resultDeferred.complete(failureResult) + } + } + + onComplete() + } + } + + private suspend fun readStream( + process: Process, + isStdErr: Boolean, + ) { + val stream = if (isStdErr) process.errorStream else process.inputStream + val builder = if (isStdErr) stderrBuilder else stdoutBuilder + BufferedReader(InputStreamReader(stream)).use { reader -> + var line = reader.readLine() + while (line != null) { + if (builder.length + line.length <= MAX_OUTPUT_BYTES) { + builder.appendLine(line) + } + val output = if (isStdErr) CommandOutput.StdErr(line) else CommandOutput.StdOut(line) + outputChannel.send(output) + line = reader.readLine() + } + } + } + + override suspend fun await(): CommandResult = resultDeferred.await() + + override fun cancel() { + process?.destroyForcibly() + outputChannel.close() + if (resultDeferred.isActive) { + resultDeferred.complete( + CommandResult.Cancelled(stdoutBuilder.toString(), stderrBuilder.toString()), + ) + } + scope.cancel() + } + + fun isRunning(): Boolean = process?.isAlive == true + + companion object { + private const val MAX_OUTPUT_BYTES = 10 * 1024 * 1024 + } +} From 6541d31954157f358ab04595400b4d27cd33f7d6 Mon Sep 17 00:00:00 2001 From: Daniel Alome Date: Thu, 1 Oct 2026 23:29:36 +0100 Subject: [PATCH 2/2] ADFA-5086: Fail a command with a bad working directory instead of throwing executeCommand passed the plugin-supplied ShellCommand.workingDirectory straight to Paths.get and File.canonicalFile, so a NUL character threw InvalidPathException and a path too long to resolve threw IOException, both out of the host service and onto the plugin's calling thread. The environment map had the same problem: ProcessBuilder rejects a NUL or '=' in a name or value with IllegalArgumentException. These now return a CommandExecution that has already completed with CommandResult.Failure ("Invalid working directory: ..." or "Invalid environment: ..."), the same shape a failed process start already reports. Permission, concurrency and outside-the-project-root checks still throw SecurityException/IllegalStateException as before. --- .../manager/services/IdeCommandServiceImpl.kt | 43 +++++++++++++-- .../services/IdeCommandServiceImplTest.kt | 55 +++++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImplTest.kt diff --git a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt index 2beb041cbf..942c56ddcd 100644 --- a/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt +++ b/plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt @@ -13,12 +13,15 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.cancel import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeout import java.io.BufferedReader import java.io.File +import java.io.IOException import java.io.InputStreamReader +import java.nio.file.InvalidPathException import java.nio.file.Path import java.nio.file.Paths import java.util.UUID @@ -46,15 +49,20 @@ class IdeCommandServiceImpl( when (spec) { is CommandSpec.ShellCommand -> { val workDir = - when { - spec.workingDirectory == null -> projectRoot - Paths.get(spec.workingDirectory).isAbsolute -> File(spec.workingDirectory) - else -> projectRoot?.let { File(it, spec.workingDirectory).canonicalFile } + try { + resolveWorkingDirectory(spec.workingDirectory, projectRoot).also(::validateWorkingDirectory) + } catch (e: InvalidPathException) { + return RejectedCommandExecution(executionId, "Invalid working directory: ${e.message}") + } catch (e: IOException) { + return RejectedCommandExecution(executionId, "Invalid working directory: ${e.message}") } - validateWorkingDirectory(workDir) ProcessBuilder(listOf(spec.executable) + spec.arguments).apply { workDir?.let { directory(it) } - environment().putAll(spec.environment) + try { + environment().putAll(spec.environment) + } catch (e: IllegalArgumentException) { + return RejectedCommandExecution(executionId, "Invalid environment: ${e.message}") + } } } @@ -121,6 +129,16 @@ class IdeCommandServiceImpl( } } + private fun resolveWorkingDirectory( + workingDirectory: String?, + projectRoot: File?, + ): File? = + when { + workingDirectory == null -> projectRoot + Paths.get(workingDirectory).isAbsolute -> File(workingDirectory) + else -> projectRoot?.let { File(it, workingDirectory).canonicalFile } + } + private fun validateWorkingDirectory(dir: File?) { if (dir == null) return val projectRoot = projectRootProvider() ?: return @@ -269,3 +287,16 @@ private class CommandExecutionImpl( private const val MAX_OUTPUT_BYTES = 10 * 1024 * 1024 } } + +private class RejectedCommandExecution( + override val executionId: String, + error: String, +) : CommandExecution { + private val result = CommandResult.Failure(-1, "", "", error, 0) + + override val output: Flow = emptyFlow() + + override suspend fun await(): CommandResult = result + + override fun cancel() = Unit +} diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImplTest.kt new file mode 100644 index 0000000000..ecbb1c891d --- /dev/null +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImplTest.kt @@ -0,0 +1,55 @@ +package com.itsaky.androidide.plugins.manager.services + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.plugins.PluginPermission +import com.itsaky.androidide.plugins.extensions.CommandResult +import com.itsaky.androidide.plugins.extensions.CommandSpec +import kotlinx.coroutines.runBlocking +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +class IdeCommandServiceImplTest { + @get:Rule + val tmp = TemporaryFolder() + + private fun resultFor(workingDirectory: String): CommandResult = + resultFor(CommandSpec.ShellCommand("true", workingDirectory = workingDirectory)) + + private fun resultFor(spec: CommandSpec): CommandResult { + val projectRoot = tmp.newFolder("project") + val service = + IdeCommandServiceImpl( + pluginId = "test.plugin", + permissions = setOf(PluginPermission.SYSTEM_COMMANDS), + projectRootProvider = { projectRoot }, + appFilesDir = tmp.newFolder("files"), + ) + val execution = service.executeCommand(spec, 5_000) + return runBlocking { execution.await() } + } + + @Test + fun `a working directory with a NUL character fails the command`() { + val result = resultFor("sub\u0000dir") + + assertThat(result).isInstanceOf(CommandResult.Failure::class.java) + assertThat((result as CommandResult.Failure).error).startsWith("Invalid working directory") + } + + @Test + fun `a working directory too long to resolve fails the command`() { + val result = resultFor("a/".repeat(3000)) + + assertThat(result).isInstanceOf(CommandResult.Failure::class.java) + assertThat((result as CommandResult.Failure).error).startsWith("Invalid working directory") + } + + @Test + fun `an environment variable with a NUL character fails the command`() { + val result = resultFor(CommandSpec.ShellCommand("true", environment = mapOf("KEY" to "a\u0000b"))) + + assertThat(result).isInstanceOf(CommandResult.Failure::class.java) + assertThat((result as CommandResult.Failure).error).startsWith("Invalid environment") + } +}