Skip to content
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.

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

Termux:Tasker Plugin Various Improvements #36

Merged

Conversation

agnostic-apollo
Copy link
Member

Hey,

Here are a bunch of improvements for Termux:Tasker. Check the new README.md for more details, some things not detailed here to prevent duplication.

  1. Arguments parsing was broken, double quotes can't be send either, like for json. The argument splitting in code was done here.

Without the java escaping, the regex is ([^"]\S*|".+?")\s*.

What that means in layman is that:

(non double quote character followed by zero or more non white space characters OR one or more characters surrounded by double quotes) followed by zero or more whitespace characters

Basically string splits on whitespaces unless enclosed in double quotes. But the second part of the OR does not check if double quotes are preceded by a backslash so to ignore them. So if you pass "some string with \" double quote", i think it will likely become two arguments some string with \ and double quote

After that it also replaces all double quotes with nothing in line 47. So technically there was no way to pass double quotes from what I saw.

I have fixed that by using ArgumentTokenizer which will parse the arguments as normally done on the shell, including surrounding with single quotes for sending literal strings, of course any single quotes inside the string that needs to be passed will have to be escaped.

  1. Since termux can run dangerous commands, there should be better security. I have added com.termux.permission.RUN_COMMAND to the FireReceiver that will be required to run ANY termux commands with the plugin from henceforth. The Tasker app has requested the permission since v5.9.3 so an update would not be required for it. Other plugin host apps will have to be updated for the plugin to continue working.

  2. Added support for absolute paths outside ~/.termux/tasker/ directory, provided that allow-external-apps property is set to true by the user in ~/.termux/termux.properties file. The permission combined with the property check provides reasonable security for apps to execute arbitrary commands. This is already done by the RUN_COMMAND intent. The benefit would be that the result of commands can be received by the sender like Tasker as well. Note that this behaviour is a bit different from RUN_COMMAND intent, it requires both to run any commands, whereas the plugin will only require the permission to run scripts in tasker directory and both to run absolute paths. This will make it easier for users to just grant the permission for scripts and they can optionally not set the property to true if they don’t want plugin apps to run arbitrary commands. If you want this behaviour changed, let me know.

  3. Added support for working directory, specially since absolute paths are now supported. It will be automatically created if inside ~/.

  4. Adding support for users to set plugin logcat log levels to help with debugging through the configuration activity options menu.

  5. The whole app is kinda refactored with safety checks everywhere, notifying the user of errors in every step, like if they are missing a permissions and if they have invalid values set or if $PREFIX can't be accessed by the plugin in case of termux-app not installed or mixing of install sources. The %errmsg variable will also be used for reporting errors to tasker while running the plugin action that occurs before command execution.

  6. I have added the PluginResultsService class that is a unified class and service to handle sending of plugin commands to the execution service, receiving the results of plugin commands back from the execution service and sending immediate or pending results back to the plugin host. The termux-app should consider importing the class and using the sendExecuteResultToResultsService() function in BackgroundJob to send exceptions in %errmsg so that plugin host apps can be notified instead of users having to check logcat and also for sending back the final result in %stdout etc.

  7. Template tasker task and scripts for running the plugin have been added in templates/.

  8. There is a kinda a related bug mentioned by @Grimler91 here which was being caused by Termux:Tasker plugin sending TaskerPlugin.Setting.RESULT_CODE_PENDING back to tasker for commands that had to be run in the terminal session and the tasker action had been set to a timeout of >0. The issue was that TermuxService didn't send any result via PendingIntent back to the plugin if EXTRA_EXECUTE_IN_BACKGROUND was false, so the plugin also never sent any result back to tasker even though tasker was expecting it and hence action timed out. I have fixed that by sending TaskerPlugin.Setting.RESULT_CODE_OK directly from the plugin if inTerminal is true. Timeout will also default to 10s now regardless of inTerminal value so that %errmsg can be returned to plugin host app, since otherwise if it were set to 0s, plugin host wouldn't wait for any errors and continued the task and user wouldn't know what happened, like if allow-external-apps is set to false and absolute path out ~/.termux/tasker/ directory is set as Executable. The users too should update their tasks and use the default 10s even with inTerminal true. The %errmsg, %stdout, %stderr and %result variables will also be cleared automatically whenever the action is run if timeout is greater than 0 to solve the issue of if multiple actions are run in the same task, then variables from previous action may still be set and get mixed in with current ones.

  9. I have updated the code to AndroidX and made other changes so it can be compiled for targetSdkVersion 29 when needed just by incrementing the number, it currently still targets 28 since termux-app isn't ready. Playstore uploads will not be possible since nov 2 deadline has passed and uploads are not planned anyways until termux-app is ready. The plugin works fine when targeting sdk 29 with the current termux-app targeting sdk 28 and also with android-10 branch targeting sdk 29.

If termux-app targets sdk 28 and termux-tasker targets sdk 29 on android 10 emulator.

The termux-tasker gives avc: granted { execute } warnings when canExecute() is called:

com.termux.tasker W/sker:background: type=1400 audit(0.0:136): avc: granted { execute } for name="tudo" dev="vdc" ino=139347 scontext=u:r:untrusted_app_27:s0:c138,c256,c512,c768 tcontext=u:object_r:app_data_file:s0:c138,c256,c512,c768 tclass=file

The termux-app gives the following warnings on execution:

W/com.termux: type=1400 audit(0.0:137): avc: granted { execute } for name="tudo" dev="vdc" ino=139347 scontext=u:r:untrusted_app_27:s0:c138,c256,c512,c768 tcontext=u:object_r:app_data_file:s0:c138,c256,c512,c768 tclass=file
W/com.termux: type=1400 audit(0.0:138): avc: granted { execute_no_trans } for path="/data/data/com.termux/files/usr/bin/tudo" dev="vdc" ino=139347 scontext=u:r:untrusted_app_27:s0:c138,c256,c512,c768 tcontext=u:object_r:app_data_file:s0:c138,c256,c512,c768 tclass=file

These are likely due to auditallow rules and just security notifications and hopefully won't get changed to denial messages in future, but who knows. ;)

If termux-app (android-10 branch) and termux-tasker both target sdk 29 on android 11 emulator, the scripts and running bash commands do execute fine, although there are following avc warnings, not sure what's causing them. The lib275.so is bash.


W/lib275.so: type=1400 audit(0.0:968): avc: denied { ioctl } for path="pipe:[244011]" dev="pipefs" ino=244011 ioctlcmd=0x540f scontext=u:r:untrusted_app_29:s0:c158,c256,c512,c768 tcontext=u:r:untrusted_app_29:s0:c158,c256,c512,c768 tclass=fifo_file permissive=0 app=com.termux.tasker

Moreover, due to apparent proot usage to run scripts, the script shebangs need to be set to #!/usr/bin/bash instead of #!/data/data/com.termux/files/usr/bin/bash, otherwise scripts fail. Is that behaviour likely to stay, since that may break current scripts of users. The Termux Environment section of the README.md would require updating too. I have set the template scripts shebangs to #!/usr/bin/... just in case.

com.termux E/termux-task: Failed running background job: [/data/data/com.termux/files/home/.termux/tasker/termux_tasker_basic_bash_test, 1, 2]
    java.io.IOException: Cannot run program "/data/data/com.termux/files/home/.termux/tasker/termux_tasker_basic_bash_test" (in directory "/data/data/com.termux/files/home"): error=13, Permission denied

So basically, since no executions are directly done in termux-tasker, the app should work fine if it targets sdk 29. Changes just have to be made on the termux-app side. Shebangs will cause problems for existing scripts though. Although, pushing an update for termux-tasker to playstore for bug fixes can be done by targeting sdk 29 and keeping termux-app at 28, since its working that way, but will require termux-app to be installed before the plugin, otherwise due to sharedUserId, sdk 29 exec restrictions kick in, so that could be troublesome.

  1. The plugin version should ideally be incremented to 0.5 since that's referenced everywhere in the README.md and templates.

  2. unrelated The termux-app BackgroundJob.BackgroundJob() and TermuxService.createTermSession() functions need to be updated for targeting sdk 29

if (cwd == null) cwd = TermuxService.HOME_PATH; needs to be changed to if (cwd == null || cwd.isEmpty()) cwd = TermuxService.HOME_PATH; since otherwise if an empty working directory string extra is passed from the RUN_COMMAND intent or the plugin, the following exception is raised due to an empty string being passed to Runtime.getRuntime().exec(progArray, env, new File(cwd));. I have fixed that from the plugin side by not passing an empty directory but this should be fixed from termux-app side as well, cause of issues with RUN_COMMAND intent and possibly others.

Failed running background job: [/data/app/~~DddU2rGhxCU5dUZToGj1lg==/com.termux-jWutjkkLdQ5-2GJ1h6qr2A==/lib/x86/lib275.so, -c, echo 1; sleep 3;]
    java.io.IOException: Cannot run program "/data/app/~~DddU2rGhxCU5dUZToGj1lg==/com.termux-jWutjkkLdQ5-2GJ1h6qr2A==/lib/x86/lib275.so" (in directory ""): error=2, No such file or directory
  1. unrelated The termux-app RunCommandService.parsePath() function needs to be fixed as per FileUtils.getExpandedTermuxPath() function in termux-tasker. The working directory should also be created automatically if inside termux home as per termux-tasker. There should also be a dedicated Wiki page added for RUN_COMMAND intent so its easier to find for users and devs instead of its brief mention in FAQ, something that shows in search results easily like Running termux commands from other apps.

…s like normally done on shells like bourne shell
Make `PluginResultsService` a unified class and service to handle sending of plugin commands to the execution service, receiving the results of plugin commands back from the execution service and sending immediate or pending results back to the plugin host.
…er`.

- Changed `ResultsService` service name to `PluginResultsService`.
- Moved `getVersionCode()` function to `PluginUtils` class.
…ger` util class.

- Added support to store `log_level` persistently in android `QueryPreferences` using `QueryPreferences` util class.
…ory, provided that `allow-external-apps` property is set to `true` by the user in `~/.termux/termux.properties` file.

- Added support for working directory.
- Adding support for users to set plugin `logcat` log levels to help with debugging through the configuration activity options menu.
- Added support to automatically clear `%errmsg`, `%stdout`, `%stderr` and `%result` variables when plugin action is run.
- Adding support for showing warnings in configuration activity and returning error messages via `%errmsg` to plugin host app on invalid plugin configuration or missing permissions or access failures.
- Updated `FireReceiver` to use `PluginResultsService` for sending commands to execution service and to send error messages to plugin host app if required.
- Updated default action timeout value to `10s` regardless of `inTerminal` value.
- Removed requirement for `TASKER_DIR` to exist for plugin configuration since absolute paths are allowed.
- Fixed `isBundleValid()` function.
- Changed `case` statements to `if` since resource IDs are non-final from gradle plugin version `5`.
- Updated library versions and switched to `AndroidX`.
@agnostic-apollo agnostic-apollo force-pushed the termux-tasker-plugin-various-improvements branch from 37b97b0 to aa13bb4 Compare December 6, 2020 14:22
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@ghost ghost merged commit a798fa4 into termux:master Dec 6, 2020
@agnostic-apollo
Copy link
Member Author

That was quick. Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant