feat: add Shizuku ADB root support for Root Proxy mode - #140
Open
nqmgaming wants to merge 1 commit into
Open
Conversation
Allows devices without Magisk/KernelSU to use Root Proxy mode via Shizuku when ADB root debugging is enabled on custom ROMs. Falls back transparently: tries libsu first, then Shizuku. Closes pass-with-high-score/discussions#139 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR integrates Shizuku, an ADB root-based privilege escalation framework, as an alternative backend to libsu for executing privileged shell commands. It adds Shizuku dependencies, creates an AIDL service for inter-process shell command execution, implements ShizukuManager for availability/permission handling, and refactors IptablesManager to support dual backends with automatic detection and failover logic. Changes
Sequence DiagramsequenceDiagram
actor User
participant App
participant ShizukuManager
participant Shizuku as Shizuku Daemon
participant ShizukuShellService
participant IptablesManager
User->>App: Enable Routing Mode
App->>IptablesManager: isRootAvailable()
IptablesManager->>IptablesManager: Try libsu (LIBSU backend)
alt libsu fails
IptablesManager->>ShizukuManager: isRootAvailable()
ShizukuManager->>Shizuku: pingBinder()
alt Shizuku unavailable
ShizukuManager-->>IptablesManager: false
IptablesManager-->>App: false
App->>App: Show "root_not_available" toast
else Shizuku available
ShizukuManager->>Shizuku: checkSelfPermission()
alt Permission not granted
ShizukuManager->>Shizuku: requestPermission()
Shizuku-->>ShizukuManager: Permission dialog shown
ShizukuManager-->>IptablesManager: false (pending)
IptablesManager-->>App: false
App->>App: Show "shizuku_permission_required" + trigger request
else Permission granted
ShizukuManager->>ShizukuManager: ensureServiceBound()
ShizukuManager->>Shizuku: bindUserService(ShizukuShellService)
Shizuku-->>ShizukuShellService: Create & bind
ShizukuShellService-->>ShizukuManager: Service bound
ShizukuManager->>ShizukuShellService: execCommand("id")
ShizukuShellService->>ShizukuShellService: Run sh -c id
ShizukuShellService-->>ShizukuManager: "0\nuid=0(...)"
ShizukuManager-->>IptablesManager: true (SHIZUKU backend selected)
IptablesManager-->>App: true
App->>IptablesManager: setupRules()
IptablesManager->>ShizukuManager: exec(iptables command)
ShizukuManager->>ShizukuShellService: execCommand(iptables...)
ShizukuShellService-->>ShizukuManager: Result
ShizukuManager-->>IptablesManager: CmdResult
IptablesManager->>App: Success
end
end
else libsu succeeds
IptablesManager-->>App: true (LIBSU backend)
App->>IptablesManager: setupRules()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 348147081e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+14
to
+16
| override fun destroy() { | ||
| // Called when Shizuku unbinds. No cleanup needed. | ||
| } |
There was a problem hiding this comment.
When Root Proxy stops, ShizukuManager.unbindService() unbinds the user service, but this service's destroy() callback is currently a no-op. Shizuku does not automatically kill user-service processes on unbind, so in this path the privileged process can remain alive after shutdown, leaving stale root state and unnecessary background resource usage until the process is killed externally. destroy() should perform cleanup and explicitly terminate the process.
Useful? React with 👍 / 👎.
Comment on lines
+25
to
+27
| val process = Runtime.getRuntime().exec(arrayOf("sh", "-c", command)) | ||
| val stdout = process.inputStream.bufferedReader().readText() | ||
| val exitCode = process.waitFor() |
There was a problem hiding this comment.
execCommand reads only stdout and then calls waitFor(), but never drains stderr. If a command writes substantial output to stderr (for example repeated iptables failures), the stderr pipe can fill and block the child process, causing waitFor() to hang and stalling Root Proxy setup/teardown while using the Shizuku backend.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/pwhs/blockads/service/IptablesManager.kt`:
- Around line 106-107: The call that disables Android Private DNS
(exec("settings put global private_dns_mode off")) is unchecked and always logs
success; update IptablesManager so the exec result/exception is checked and
treat failure as fatal: in the method where exec is invoked (the code path that
leads into setupRules()/isActive()), capture the exit code or thrown error from
the exec invocation, log the real error via Timber.e, and return/throw to make
setupRules() fail fast (so setupRules() returns false or propagate an exception)
instead of unconditionally logging success and continuing; ensure isActive()
remains unchanged but relies on setupRules()'s failure to prevent further
activation.
- Around line 77-84: The current execBatch function collapses Shizuku commands
with commands.joinToString("; ") so only the last command's exit code is
observed; change execBatch to iterate when activeBackend == ShellBackend.SHIZUKU
and call ShizukuManager.exec for each command individually,
collecting/aggregating success/failure (e.g., track boolean or return status) so
callers like teardownRules see failures from earlier iptables cleanup commands
rather than being masked by a final settings put global command; keep the LIBSU
branch unchanged (Shell.cmd(*commands.toTypedArray()).exec()) and ensure
execBatch returns/propagates an aggregated result or throws on failure per
project convention.
In `@app/src/main/java/app/pwhs/blockads/service/RootProxyService.kt`:
- Around line 261-264: StopProxy only unbinds Shizuku in stopProxy(), leaving
other teardown paths inconsistent; add a single private helper (e.g.,
cleanupShizukuBinding or ensureShizukuUnbound) that checks
IptablesManager.activeBackend == IptablesManager.ShellBackend.SHIZUKU and calls
ShizukuManager.unbindService() and resets IptablesManager.activeBackend to a
safe default, then invoke that helper from stopProxy(), restartProxy(),
onDestroy(), and onTaskRemoved() to consolidate cleanup and avoid stale
bindings.
In `@app/src/main/java/app/pwhs/blockads/service/ShizukuManager.kt`:
- Around line 115-118: The parser in ShizukuManager.kt currently treats all
payload lines as stdout (ShellResult(..., out=output, err=emptyList())), so
failures from ShizukuShellService (which return "-1\n<message>") lose their
error text; change the construction so that after parsing the first line as
exitCode and the rest as payload you set ShellResult(success = exitCode == 0,
out = if (exitCode == 0) payload else emptyList(), err = if (exitCode == 0)
emptyList() else payload) so error payloads appear in ShellResult.err for failed
commands.
In `@app/src/main/java/app/pwhs/blockads/service/ShizukuShellService.kt`:
- Around line 23-31: The execCommand function currently only drains
process.inputStream which can block if stderr fills; update execCommand to
consume stderr as well before waitFor by either using a ProcessBuilder (replace
Runtime.getRuntime().exec call with ProcessBuilder(arrayOf("sh","-c",
command))).redirectErrorStream(true).start() to merge stderr into stdout, or if
you keep Runtime.exec, spawn a concurrent reader for process.errorStream (e.g.,
a background thread or async read) and readText() into a variable, then call
process.waitFor() and return combined exit code, stdout and stderr; reference
the execCommand method to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0273fc96-fa0e-42bf-b83b-e213b538e074
📒 Files selected for processing (11)
app/build.gradle.ktsapp/proguard-rules.proapp/src/main/AndroidManifest.xmlapp/src/main/aidl/app/pwhs/blockads/service/IShizukuShellService.aidlapp/src/main/java/app/pwhs/blockads/service/IptablesManager.ktapp/src/main/java/app/pwhs/blockads/service/RootProxyService.ktapp/src/main/java/app/pwhs/blockads/service/ShizukuManager.ktapp/src/main/java/app/pwhs/blockads/service/ShizukuShellService.ktapp/src/main/java/app/pwhs/blockads/ui/settings/SettingsViewModel.ktapp/src/main/res/values/strings.xmlgradle/libs.versions.toml
Comment on lines
+77
to
+84
| private fun execBatch(commands: List<String>) { | ||
| when (activeBackend) { | ||
| ShellBackend.LIBSU -> Shell.cmd(*commands.toTypedArray()).exec() | ||
| ShellBackend.SHIZUKU -> { | ||
| // Shizuku doesn't support batch — join with ';' | ||
| ShizukuManager.exec(commands.joinToString("; ")) | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't collapse Shizuku batches into one ;-joined command.
Only the last exit code survives here. In teardownRules(), a successful final settings put global private_dns_mode opportunistic will mask earlier iptables cleanup failures, yet the caller still logs full success. Execute each command separately and aggregate the results instead.
Suggested fix
- private fun execBatch(commands: List<String>) {
- when (activeBackend) {
- ShellBackend.LIBSU -> Shell.cmd(*commands.toTypedArray()).exec()
- ShellBackend.SHIZUKU -> {
- // Shizuku doesn't support batch — join with ';'
- ShizukuManager.exec(commands.joinToString("; "))
- }
- }
+ private fun execBatch(commands: List<String>): List<CmdResult> {
+ return commands.map(::exec)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/pwhs/blockads/service/IptablesManager.kt` around lines
77 - 84, The current execBatch function collapses Shizuku commands with
commands.joinToString("; ") so only the last command's exit code is observed;
change execBatch to iterate when activeBackend == ShellBackend.SHIZUKU and call
ShizukuManager.exec for each command individually, collecting/aggregating
success/failure (e.g., track boolean or return status) so callers like
teardownRules see failures from earlier iptables cleanup commands rather than
being masked by a final settings put global command; keep the LIBSU branch
unchanged (Shell.cmd(*commands.toTypedArray()).exec()) and ensure execBatch
returns/propagates an aggregated result or throws on failure per project
convention.
Comment on lines
+106
to
107
| exec("settings put global private_dns_mode off") | ||
| Timber.d("Disabled Android Private DNS (forced plain DNS mode)") |
There was a problem hiding this comment.
Fail fast when Private DNS could not be disabled.
This command is a prerequisite for interception, but its result is ignored and the success log is unconditional. If settings put global private_dns_mode off is rejected, setupRules() can still return true because isActive() only checks the iptables chain.
Suggested fix
- exec("settings put global private_dns_mode off")
- Timber.d("Disabled Android Private DNS (forced plain DNS mode)")
+ val privateDnsResult = exec("settings put global private_dns_mode off")
+ if (!privateDnsResult.isSuccess) {
+ Timber.e(
+ "Failed to disable Android Private DNS: err=${privateDnsResult.err} out=${privateDnsResult.out}"
+ )
+ return false
+ }
+ Timber.d("Disabled Android Private DNS (forced plain DNS mode)")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| exec("settings put global private_dns_mode off") | |
| Timber.d("Disabled Android Private DNS (forced plain DNS mode)") | |
| val privateDnsResult = exec("settings put global private_dns_mode off") | |
| if (!privateDnsResult.isSuccess) { | |
| Timber.e( | |
| "Failed to disable Android Private DNS: err=${privateDnsResult.err} out=${privateDnsResult.out}" | |
| ) | |
| return false | |
| } | |
| Timber.d("Disabled Android Private DNS (forced plain DNS mode)") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/pwhs/blockads/service/IptablesManager.kt` around lines
106 - 107, The call that disables Android Private DNS (exec("settings put global
private_dns_mode off")) is unchecked and always logs success; update
IptablesManager so the exec result/exception is checked and treat failure as
fatal: in the method where exec is invoked (the code path that leads into
setupRules()/isActive()), capture the exit code or thrown error from the exec
invocation, log the real error via Timber.e, and return/throw to make
setupRules() fail fast (so setupRules() returns false or propagate an exception)
instead of unconditionally logging success and continuing; ensure isActive()
remains unchanged but relies on setupRules()'s failure to prevent further
activation.
Comment on lines
+261
to
+264
| // Release Shizuku shell service if it was used | ||
| if (IptablesManager.activeBackend == IptablesManager.ShellBackend.SHIZUKU) { | ||
| ShizukuManager.unbindService() | ||
| } |
There was a problem hiding this comment.
Unbind logic is incomplete across teardown paths.
Line 261-Line 264 unbinds Shizuku only in stopProxy(), but teardown also happens in restartProxy() (Line 317), onDestroy() (Line 361), and onTaskRemoved() (Line 368) without the same cleanup. This can leave stale Shizuku service bindings and inconsistent backend state.
Suggested consolidation fix
private fun stopProxy(showPausedNotification: Boolean = false) {
@@
- // Teardown iptables rules (critical — prevents internet loss)
- IptablesManager.teardownRules()
-
- // Release Shizuku shell service if it was used
- if (IptablesManager.activeBackend == IptablesManager.ShellBackend.SHIZUKU) {
- ShizukuManager.unbindService()
- }
+ teardownRootBackend()
@@
}
+ private fun teardownRootBackend() {
+ IptablesManager.teardownRules()
+ if (IptablesManager.activeBackend == IptablesManager.ShellBackend.SHIZUKU) {
+ ShizukuManager.unbindService()
+ }
+ }
+
@@
- IptablesManager.teardownRules()
+ teardownRootBackend()
@@
- IptablesManager.teardownRules()
+ teardownRootBackend()
@@
- IptablesManager.teardownRules()
+ teardownRootBackend()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/pwhs/blockads/service/RootProxyService.kt` around lines
261 - 264, StopProxy only unbinds Shizuku in stopProxy(), leaving other teardown
paths inconsistent; add a single private helper (e.g., cleanupShizukuBinding or
ensureShizukuUnbound) that checks IptablesManager.activeBackend ==
IptablesManager.ShellBackend.SHIZUKU and calls ShizukuManager.unbindService()
and resets IptablesManager.activeBackend to a safe default, then invoke that
helper from stopProxy(), restartProxy(), onDestroy(), and onTaskRemoved() to
consolidate cleanup and avoid stale bindings.
Comment on lines
+115
to
+118
| val lines = raw.lines() | ||
| val exitCode = lines.firstOrNull()?.toIntOrNull() ?: -1 | ||
| val output = lines.drop(1).filter { it.isNotEmpty() } | ||
| ShellResult(exitCode == 0, output, emptyList()) |
There was a problem hiding this comment.
Preserve failure payload in ShellResult.err.
app/src/main/java/app/pwhs/blockads/service/ShizukuShellService.kt:23-32 returns -1\n<message> on failures, but this parser always stores the payload in out. Downstream callers then see err=[] for failed Shizuku commands, which makes paths like the IPv6 warning log lose the actual reason.
Suggested fix
return try {
val raw = shellService!!.execCommand(command)
val lines = raw.lines()
val exitCode = lines.firstOrNull()?.toIntOrNull() ?: -1
- val output = lines.drop(1).filter { it.isNotEmpty() }
- ShellResult(exitCode == 0, output, emptyList())
+ val payload = lines.drop(1).filter { it.isNotEmpty() }
+ if (exitCode == 0) {
+ ShellResult(true, payload, emptyList())
+ } else {
+ ShellResult(false, emptyList(), payload.ifEmpty {
+ listOf("Command failed with exit code $exitCode")
+ })
+ }
} catch (e: Exception) {
Timber.e(e, "Shizuku exec failed: %s", command)
shellService = null // Force rebind on next call📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| val lines = raw.lines() | |
| val exitCode = lines.firstOrNull()?.toIntOrNull() ?: -1 | |
| val output = lines.drop(1).filter { it.isNotEmpty() } | |
| ShellResult(exitCode == 0, output, emptyList()) | |
| val lines = raw.lines() | |
| val exitCode = lines.firstOrNull()?.toIntOrNull() ?: -1 | |
| val payload = lines.drop(1).filter { it.isNotEmpty() } | |
| if (exitCode == 0) { | |
| ShellResult(true, payload, emptyList()) | |
| } else { | |
| ShellResult(false, emptyList(), payload.ifEmpty { | |
| listOf("Command failed with exit code $exitCode") | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/pwhs/blockads/service/ShizukuManager.kt` around lines
115 - 118, The parser in ShizukuManager.kt currently treats all payload lines as
stdout (ShellResult(..., out=output, err=emptyList())), so failures from
ShizukuShellService (which return "-1\n<message>") lose their error text; change
the construction so that after parsing the first line as exitCode and the rest
as payload you set ShellResult(success = exitCode == 0, out = if (exitCode == 0)
payload else emptyList(), err = if (exitCode == 0) emptyList() else payload) so
error payloads appear in ShellResult.err for failed commands.
Comment on lines
+23
to
+31
| override fun execCommand(command: String): String { | ||
| return try { | ||
| val process = Runtime.getRuntime().exec(arrayOf("sh", "-c", command)) | ||
| val stdout = process.inputStream.bufferedReader().readText() | ||
| val exitCode = process.waitFor() | ||
| "$exitCode\n$stdout" | ||
| } catch (e: Exception) { | ||
| "-1\n${e.message}" | ||
| } |
There was a problem hiding this comment.
Consume stderr before waiting on the process.
iptables/settings failures usually write to stderr. This implementation only drains stdout, so a noisy failure can block on a full stderr buffer and hang the binder call, and the actual error text is lost. Please merge or read both streams before waitFor().
Suggested fix
override fun execCommand(command: String): String {
return try {
- val process = Runtime.getRuntime().exec(arrayOf("sh", "-c", command))
- val stdout = process.inputStream.bufferedReader().readText()
- val exitCode = process.waitFor()
- "$exitCode\n$stdout"
+ val process = ProcessBuilder("sh", "-c", command)
+ .redirectErrorStream(true)
+ .start()
+ val output = process.inputStream.bufferedReader().use { it.readText() }
+ val exitCode = process.waitFor()
+ "$exitCode\n$output"
} catch (e: Exception) {
"-1\n${e.message}"
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| override fun execCommand(command: String): String { | |
| return try { | |
| val process = Runtime.getRuntime().exec(arrayOf("sh", "-c", command)) | |
| val stdout = process.inputStream.bufferedReader().readText() | |
| val exitCode = process.waitFor() | |
| "$exitCode\n$stdout" | |
| } catch (e: Exception) { | |
| "-1\n${e.message}" | |
| } | |
| override fun execCommand(command: String): String { | |
| return try { | |
| val process = ProcessBuilder("sh", "-c", command) | |
| .redirectErrorStream(true) | |
| .start() | |
| val output = process.inputStream.bufferedReader().use { it.readText() } | |
| val exitCode = process.waitFor() | |
| "$exitCode\n$output" | |
| } catch (e: Exception) { | |
| "-1\n${e.message}" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/app/pwhs/blockads/service/ShizukuShellService.kt` around
lines 23 - 31, The execCommand function currently only drains
process.inputStream which can block if stderr fills; update execCommand to
consume stderr as well before waitFor by either using a ProcessBuilder (replace
Runtime.getRuntime().exec call with ProcessBuilder(arrayOf("sh","-c",
command))).redirectErrorStream(true).start() to merge stderr into stdout, or if
you keep Runtime.exec, spawn a concurrent reader for process.errorStream (e.g.,
a background thread or async read) and readText() into a variable, then call
process.waitFor() and return combined exit code, stdout and stderr; reference
the execCommand method to locate the change.
parayou1978
approved these changes
Jun 6, 2026
parayou1978
approved these changes
Jun 7, 2026
parayou1978
approved these changes
Jun 7, 2026
parayou1978
approved these changes
Jun 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows devices without Magisk/KernelSU to use Root Proxy mode via Shizuku when ADB root debugging is enabled on custom ROMs. Falls back transparently: tries libsu first, then Shizuku.
Closes pass-with-high-score/discussions#139
Summary by CodeRabbit