Find documentation for termux screen operator#107
Conversation
There was a problem hiding this comment.
Review Summary
Identified 5 critical defects that block merge:
Security Issues (1)
- Missing permission check in Termux command execution allows unauthorized command execution
Crash Risks (2)
- Race condition with service instance and handler access during Termux callbacks
- Missing accessibility node recycling causes memory exhaustion
Logic Errors (2)
- Broadcast receiver memory leak on error paths
- Integer overflow in delay millisecond conversion
All issues have actionable fixes with code suggestions provided.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| private class TermuxResultReceiver(private val appContext: Context) : android.content.BroadcastReceiver() { | ||
| override fun onReceive(context: Context?, intent: Intent?) { | ||
| fun unregisterSelf() { | ||
| try { | ||
| appContext.unregisterReceiver(this) | ||
| Log.i(TAG, "Termux result receiver unregistered") | ||
| } catch (t: Throwable) { | ||
| Log.w(TAG, "Failed to unregister Termux result receiver", t) | ||
| } | ||
| } | ||
| if (intent == null) { | ||
| Log.w(TAG, "Termux result receiver invoked with null intent") | ||
| unregisterSelf() | ||
| return | ||
| } | ||
| val resultBundle = intent.getBundleExtra("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE") | ||
| ?: intent.getBundleExtra("result") | ||
|
|
||
| val extras = intent.extras | ||
| val stdout = sequenceOf( | ||
| resultBundle?.getString("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_STDOUT"), | ||
| resultBundle?.getString("stdout"), | ||
| extras?.getString("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_STDOUT"), | ||
| extras?.getString("stdout") | ||
| ).firstOrNull { !it.isNullOrBlank() }.orEmpty() | ||
| val stderr = sequenceOf( | ||
| resultBundle?.getString("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_STDERR"), | ||
| resultBundle?.getString("stderr"), | ||
| extras?.getString("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_STDERR"), | ||
| extras?.getString("stderr") | ||
| ).firstOrNull { !it.isNullOrBlank() }.orEmpty() | ||
| val exitCode = when { | ||
| resultBundle?.containsKey("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_EXIT_CODE") == true -> { | ||
| resultBundle.getInt("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_EXIT_CODE", Int.MIN_VALUE) | ||
| } | ||
| resultBundle?.containsKey("exitCode") == true -> resultBundle.getInt("exitCode", Int.MIN_VALUE) | ||
| extras?.containsKey("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_EXIT_CODE") == true -> { | ||
| extras.getInt("com.termux.app.extra.TERMUX_SERVICE.EXTRA_PLUGIN_RESULT_BUNDLE_EXIT_CODE", Int.MIN_VALUE) | ||
| } | ||
| extras?.containsKey("exitCode") == true -> extras.getInt("exitCode", Int.MIN_VALUE) | ||
| else -> Int.MIN_VALUE | ||
| } | ||
|
|
||
| val resultKeys = resultBundle?.keySet()?.joinToString().orEmpty() | ||
| val extraKeys = extras?.keySet()?.joinToString().orEmpty() | ||
| Log.i(TAG, "Termux result received: exitCode=$exitCode stdoutLen=${stdout.length} stderrLen=${stderr.length} bundleKeys=$resultKeys extraKeys=$extraKeys") | ||
|
|
||
| val hasKnownResult = stdout.isNotBlank() || stderr.isNotBlank() || exitCode != Int.MIN_VALUE | ||
| if (!hasKnownResult) { | ||
| val rawExtrasDump = extras?.keySet()?.joinToString("\n") { key -> "$key=${extras.get(key)}" }.orEmpty().trim() | ||
| if (rawExtrasDump.isBlank()) { | ||
| Log.w(TAG, "Ignoring Termux callback without stdout/stderr/exitCode and no readable extras.") | ||
| unregisterSelf() | ||
| return | ||
| } | ||
| Log.w(TAG, "Termux callback missing standard stdout/stderr/exitCode fields; falling back to raw extras dump for AI handoff.") | ||
| TermuxOutputPreferences.appendOutput(appContext, "Termux callback raw extras:\n$rawExtrasDump") | ||
| mainHandler.post { | ||
| MainActivity.getInstance()?.updateStatusMessage("Termux raw result captured", false) | ||
| } | ||
| serviceInstance?.handler?.post { | ||
| Log.d(TAG, "Termux raw callback captured, scheduling next command processing.") | ||
| serviceInstance?.scheduleNextCommandProcessing() | ||
| } | ||
| unregisterSelf() | ||
| return | ||
| } | ||
|
|
||
| val combined = buildString { | ||
| append("Termux finished") | ||
| if (exitCode != Int.MIN_VALUE) { | ||
| append(" (exit=") | ||
| append(exitCode) | ||
| append(")") | ||
| } | ||
| if (stdout.isNotBlank()) { | ||
| append("\nstdout:\n") | ||
| append(stdout) | ||
| } | ||
| if (stderr.isNotBlank()) { | ||
| append("\nstderr:\n") | ||
| append(stderr) | ||
| } | ||
| } | ||
|
|
||
| val aiRelevantOutput = combined.trim() | ||
| if (aiRelevantOutput.isNotBlank()) { | ||
| TermuxOutputPreferences.appendOutput(appContext, aiRelevantOutput) | ||
| Log.i(TAG, "Stored Termux output for next screenshot bubble. chars=${aiRelevantOutput.length}") | ||
| } | ||
|
|
||
| mainHandler.post { | ||
| MainActivity.getInstance()?.updateStatusMessage("Termux stream start", false) | ||
| } | ||
| combined.lineSequence().forEachIndexed { idx, line -> | ||
| val framed = "Termux[$idx]: $line" | ||
| Log.d(TAG, framed) | ||
| mainHandler.post { | ||
| MainActivity.getInstance()?.updateStatusMessage(framed, false) | ||
| } | ||
| } | ||
|
|
||
| serviceInstance?.handler?.post { | ||
| Log.d(TAG, "Termux result received, scheduling next command processing.") | ||
| serviceInstance?.scheduleNextCommandProcessing() | ||
| } | ||
| unregisterSelf() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Potential race condition with handler access. The static mainHandler is accessed from multiple threads, and serviceInstance?.handler is accessed without null-safety in the inner class. If the service is destroyed while Termux callbacks are executing, accessing serviceInstance?.handler?.post at lines 715 and 757 could cause crashes.
| pendingScreenshotDelayMillis = command.seconds | ||
| .coerceAtLeast(0L) | ||
| .coerceAtMost(Long.MAX_VALUE / 1000L) * 1000L | ||
| Log.d(TAG, "Command.Wait: Delaying the next takeScreenshot command by ${command.seconds} seconds.") | ||
| showToast("Delaying next screenshot by ${command.seconds} seconds", false) | ||
| false |
There was a problem hiding this comment.
🛑 Logic Error: Integer overflow causes millisecond conversion to fail. The multiplication command.seconds * 1000L at line 248 can overflow for values near Long.MAX_VALUE / 1000L, but this check occurs after coercion. For input 9223372036854775L seconds, the overflow wraps to a negative value, causing the delay to be treated as zero or negative.1
| pendingScreenshotDelayMillis = command.seconds | |
| .coerceAtLeast(0L) | |
| .coerceAtMost(Long.MAX_VALUE / 1000L) * 1000L | |
| Log.d(TAG, "Command.Wait: Delaying the next takeScreenshot command by ${command.seconds} seconds.") | |
| showToast("Delaying next screenshot by ${command.seconds} seconds", false) | |
| false | |
| is Command.Wait -> { | |
| val secondsClamped = command.seconds.coerceAtLeast(0L).coerceAtMost(Long.MAX_VALUE / 1000L) | |
| pendingScreenshotDelayMillis = secondsClamped * 1000L | |
| Log.d(TAG, "Command.Wait: Delaying the next takeScreenshot command by $secondsClamped seconds.") | |
| showToast("Delaying next screenshot by $secondsClamped seconds", false) | |
| false | |
| } |
Footnotes
-
CWE-190: Integer Overflow or Wraparound - https://cwe.mitre.org/data/definitions/190.html ↩
| is Command.TermuxCommand -> { | ||
| executeAsyncCommandAction( | ||
| logMessage = "Executing Termux command: ${command.command}", | ||
| toastMessage = "Executing Termux command..." | ||
| ) { | ||
| executeTermuxCommand(command.command) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛑 Security Vulnerability: Missing permission check exposes Termux command execution. The permission is only checked during screenshot commands, not when Termux commands are executed. An attacker could queue Termux commands that bypass the permission guard at line 434 by avoiding screenshot commands.1
| is Command.TermuxCommand -> { | |
| executeAsyncCommandAction( | |
| logMessage = "Executing Termux command: ${command.command}", | |
| toastMessage = "Executing Termux command..." | |
| ) { | |
| executeTermuxCommand(command.command) | |
| } | |
| } | |
| is Command.TermuxCommand -> { | |
| if (!isTermuxRunCommandPermissionGranted()) { | |
| val denialCount = TermuxFeedbackPreferences.incrementPermissionDenialCount(applicationContext) | |
| if (denialCount >= 2) { | |
| showToast("Enable Termux permissions in the Android settings", true) | |
| } | |
| Log.w(TAG, "Blocking Termux command execution because Termux RUN_COMMAND permission is not granted.") | |
| false | |
| } else { | |
| TermuxFeedbackPreferences.resetPermissionDenialCount(applicationContext) | |
| executeAsyncCommandAction( | |
| logMessage = "Executing Termux command: ${command.command}", | |
| toastMessage = "Executing Termux command..." | |
| ) { | |
| executeTermuxCommand(command.command) | |
| } | |
| } | |
| } |
Footnotes
-
CWE-862: Missing Authorization - https://cwe.mitre.org/data/definitions/862.html ↩
| private fun tryPerformScrollableNodeAction(action: Int): Boolean { | ||
| refreshRootNode() | ||
| val root = rootNode ?: return false | ||
| val queue = ArrayDeque<AccessibilityNodeInfo>() | ||
| queue.add(root) | ||
| while (queue.isNotEmpty()) { | ||
| val node = queue.removeFirst() | ||
| if (node.isScrollable && node.performAction(action)) { | ||
| return true | ||
| } | ||
| for (i in 0 until node.childCount) { | ||
| node.getChild(i)?.let(queue::add) | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
🛑 Crash Risk: Missing node recycling causes resource exhaustion. The tryPerformScrollableNodeAction function adds nodes to a queue without recycling them after use. This leaks accessibility node references, eventually causing OutOfMemoryError or system-wide accessibility service crashes.
| private fun tryPerformScrollableNodeAction(action: Int): Boolean { | |
| refreshRootNode() | |
| val root = rootNode ?: return false | |
| val queue = ArrayDeque<AccessibilityNodeInfo>() | |
| queue.add(root) | |
| while (queue.isNotEmpty()) { | |
| val node = queue.removeFirst() | |
| if (node.isScrollable && node.performAction(action)) { | |
| return true | |
| } | |
| for (i in 0 until node.childCount) { | |
| node.getChild(i)?.let(queue::add) | |
| } | |
| } | |
| return false | |
| } | |
| private fun tryPerformScrollableNodeAction(action: Int): Boolean { | |
| refreshRootNode() | |
| val root = rootNode ?: return false | |
| val queue = ArrayDeque<AccessibilityNodeInfo>() | |
| queue.add(root) | |
| try { | |
| while (queue.isNotEmpty()) { | |
| val node = queue.removeFirst() | |
| try { | |
| if (node.isScrollable && node.performAction(action)) { | |
| return true | |
| } | |
| for (i in 0 until node.childCount) { | |
| node.getChild(i)?.let(queue::add) | |
| } | |
| } finally { | |
| if (node != root) { | |
| node.recycle() | |
| } | |
| } | |
| } | |
| } finally { | |
| queue.forEach { if (it != root) it.recycle() } | |
| } | |
| return false | |
| } |
| val callbackReceiver = TermuxResultReceiver(applicationContext) | ||
| try { | ||
| applicationContext.registerReceiver(callbackReceiver, android.content.IntentFilter(callbackAction), Context.RECEIVER_NOT_EXPORTED) | ||
| Log.i(TAG, "Registered Termux result receiver for action=$callbackAction") | ||
| } catch (t: Throwable) { | ||
| Log.e(TAG, "Failed to register Termux result receiver", t) | ||
| } | ||
|
|
||
| val intent = Intent("com.termux.RUN_COMMAND").apply { | ||
| `package` = termuxPackage | ||
| setClassName(termuxPackage, runCommandServiceClass) | ||
| putExtra("com.termux.RUN_COMMAND_PATH", "/data/data/com.termux/files/usr/bin/bash") | ||
| putExtra("com.termux.RUN_COMMAND_ARGUMENTS", arrayOf("-lc", trimmedCommand)) | ||
| putExtra("com.termux.RUN_COMMAND_WORKDIR", "/data/data/com.termux/files/home") | ||
| putExtra("com.termux.RUN_COMMAND_BACKGROUND", false) | ||
| putExtra("com.termux.RUN_COMMAND_SESSION_ACTION", 1) | ||
| putExtra("com.termux.RUN_COMMAND_RUNNER", "app-shell") | ||
| putExtra("com.termux.RUN_COMMAND_PENDING_INTENT", pendingResultIntent) | ||
| putExtra("com.termux.RUN_COMMAND_BACKGROUND_CUSTOM_LOG_LEVEL", 0) | ||
| putExtra("com.termux.RUN_COMMAND_RETURN_STDOUT", true) | ||
| putExtra("com.termux.RUN_COMMAND_RETURN_STDERR", true) | ||
| } | ||
|
|
||
| Log.i( | ||
| TAG, | ||
| "Dispatching Termux RUN_COMMAND with path=${intent.getStringExtra("com.termux.RUN_COMMAND_PATH")}, " + | ||
| "workdir=${intent.getStringExtra("com.termux.RUN_COMMAND_WORKDIR")}, " + | ||
| "background=${intent.getBooleanExtra("com.termux.RUN_COMMAND_BACKGROUND", false)}, " + | ||
| "runner=${intent.getStringExtra("com.termux.RUN_COMMAND_RUNNER")}, " + | ||
| "argsCount=${intent.getStringArrayExtra("com.termux.RUN_COMMAND_ARGUMENTS")?.size ?: 0}" | ||
| ) | ||
|
|
||
| try { | ||
| startService(intent) | ||
| Log.i(TAG, "Termux command dispatch succeeded.") | ||
| } catch (se: SecurityException) { | ||
| Log.e(TAG, "Failed to dispatch Termux command due to security restriction. Check Termux RUN_COMMAND permission grant.", se) | ||
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | ||
| } catch (t: Throwable) { | ||
| Log.e(TAG, "Failed to dispatch Termux command", t) | ||
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: Broadcast receiver is never unregistered on error paths. If startService(intent) at line 644 throws an exception after the receiver is registered at line 613, the receiver remains registered indefinitely, causing memory leaks and potential crashes on subsequent calls.1
| val callbackReceiver = TermuxResultReceiver(applicationContext) | |
| try { | |
| applicationContext.registerReceiver(callbackReceiver, android.content.IntentFilter(callbackAction), Context.RECEIVER_NOT_EXPORTED) | |
| Log.i(TAG, "Registered Termux result receiver for action=$callbackAction") | |
| } catch (t: Throwable) { | |
| Log.e(TAG, "Failed to register Termux result receiver", t) | |
| } | |
| val intent = Intent("com.termux.RUN_COMMAND").apply { | |
| `package` = termuxPackage | |
| setClassName(termuxPackage, runCommandServiceClass) | |
| putExtra("com.termux.RUN_COMMAND_PATH", "/data/data/com.termux/files/usr/bin/bash") | |
| putExtra("com.termux.RUN_COMMAND_ARGUMENTS", arrayOf("-lc", trimmedCommand)) | |
| putExtra("com.termux.RUN_COMMAND_WORKDIR", "/data/data/com.termux/files/home") | |
| putExtra("com.termux.RUN_COMMAND_BACKGROUND", false) | |
| putExtra("com.termux.RUN_COMMAND_SESSION_ACTION", 1) | |
| putExtra("com.termux.RUN_COMMAND_RUNNER", "app-shell") | |
| putExtra("com.termux.RUN_COMMAND_PENDING_INTENT", pendingResultIntent) | |
| putExtra("com.termux.RUN_COMMAND_BACKGROUND_CUSTOM_LOG_LEVEL", 0) | |
| putExtra("com.termux.RUN_COMMAND_RETURN_STDOUT", true) | |
| putExtra("com.termux.RUN_COMMAND_RETURN_STDERR", true) | |
| } | |
| Log.i( | |
| TAG, | |
| "Dispatching Termux RUN_COMMAND with path=${intent.getStringExtra("com.termux.RUN_COMMAND_PATH")}, " + | |
| "workdir=${intent.getStringExtra("com.termux.RUN_COMMAND_WORKDIR")}, " + | |
| "background=${intent.getBooleanExtra("com.termux.RUN_COMMAND_BACKGROUND", false)}, " + | |
| "runner=${intent.getStringExtra("com.termux.RUN_COMMAND_RUNNER")}, " + | |
| "argsCount=${intent.getStringArrayExtra("com.termux.RUN_COMMAND_ARGUMENTS")?.size ?: 0}" | |
| ) | |
| try { | |
| startService(intent) | |
| Log.i(TAG, "Termux command dispatch succeeded.") | |
| } catch (se: SecurityException) { | |
| Log.e(TAG, "Failed to dispatch Termux command due to security restriction. Check Termux RUN_COMMAND permission grant.", se) | |
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | |
| } catch (t: Throwable) { | |
| Log.e(TAG, "Failed to dispatch Termux command", t) | |
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | |
| } | |
| } | |
| val callbackReceiver = TermuxResultReceiver(applicationContext) | |
| var receiverRegistered = false | |
| try { | |
| applicationContext.registerReceiver(callbackReceiver, android.content.IntentFilter(callbackAction), Context.RECEIVER_NOT_EXPORTED) | |
| receiverRegistered = true | |
| Log.i(TAG, "Registered Termux result receiver for action=$callbackAction") | |
| } catch (t: Throwable) { | |
| Log.e(TAG, "Failed to register Termux result receiver", t) | |
| return | |
| } | |
| val intent = Intent("com.termux.RUN_COMMAND").apply { | |
| `package` = termuxPackage | |
| setClassName(termuxPackage, runCommandServiceClass) | |
| putExtra("com.termux.RUN_COMMAND_PATH", "/data/data/com.termux/files/usr/bin/bash") | |
| putExtra("com.termux.RUN_COMMAND_ARGUMENTS", arrayOf("-lc", trimmedCommand)) | |
| putExtra("com.termux.RUN_COMMAND_WORKDIR", "/data/data/com.termux/files/home") | |
| putExtra("com.termux.RUN_COMMAND_BACKGROUND", false) | |
| putExtra("com.termux.RUN_COMMAND_SESSION_ACTION", 1) | |
| putExtra("com.termux.RUN_COMMAND_RUNNER", "app-shell") | |
| putExtra("com.termux.RUN_COMMAND_PENDING_INTENT", pendingResultIntent) | |
| putExtra("com.termux.RUN_COMMAND_BACKGROUND_CUSTOM_LOG_LEVEL", 0) | |
| putExtra("com.termux.RUN_COMMAND_RETURN_STDOUT", true) | |
| putExtra("com.termux.RUN_COMMAND_RETURN_STDERR", true) | |
| } | |
| Log.i( | |
| TAG, | |
| "Dispatching Termux RUN_COMMAND with path=${intent.getStringExtra("com.termux.RUN_COMMAND_PATH")}, " + | |
| "workdir=${intent.getStringExtra("com.termux.RUN_COMMAND_WORKDIR")}, " + | |
| "background=${intent.getBooleanExtra("com.termux.RUN_COMMAND_BACKGROUND", false)}, " + | |
| "runner=${intent.getStringExtra("com.termux.RUN_COMMAND_RUNNER")}, " + | |
| "argsCount=${intent.getStringArrayExtra("com.termux.RUN_COMMAND_ARGUMENTS")?.size ?: 0}" | |
| ) | |
| try { | |
| startService(intent) | |
| Log.i(TAG, "Termux command dispatch succeeded.") | |
| } catch (se: SecurityException) { | |
| Log.e(TAG, "Failed to dispatch Termux command due to security restriction. Check Termux RUN_COMMAND permission grant.", se) | |
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | |
| if (receiverRegistered) { | |
| try { | |
| applicationContext.unregisterReceiver(callbackReceiver) | |
| } catch (t: Throwable) { | |
| Log.w(TAG, "Failed to unregister receiver after security exception", t) | |
| } | |
| } | |
| } catch (t: Throwable) { | |
| Log.e(TAG, "Failed to dispatch Termux command", t) | |
| TermuxFeedbackPreferences.markTermuxNotFound(applicationContext) | |
| if (receiverRegistered) { | |
| try { | |
| applicationContext.unregisterReceiver(callbackReceiver) | |
| } catch (t: Throwable) { | |
| Log.w(TAG, "Failed to unregister receiver after dispatch failure", t) | |
| } | |
| } | |
| } |
Footnotes
-
CWE-404: Improper Resource Shutdown or Release - https://cwe.mitre.org/data/definitions/404.html ↩
No description provided.