Repository navigation
feat(android): add the Android SDK under SDK/Android - #30
Conversation
Cherry-picked from feature/android-poc (50f232d) without its Android/ preview SDK, which SDK/Android/ replaces. The Mac app connects to one ADB-forwarded Unix socket named by script/android-connect, checks that the socket and its directory belong to the user, and rejects a hello from any app other than the expected one.
SDK/Android is the Gradle root for necto-core (plain JVM: models, transport, runtime and the shared plugins), necto-android (the ADB listener, identity, DataStore, process metrics, UI Control), necto-okhttp and a sample that ports ExampleApp. The package root and Maven group are im.toss.necto. In a debuggable build the app binds an abstract Unix socket under a random name, writes the name to no_backup/necto-endpoint and accepts only adbd's uid, so no network port is opened and no permission is declared. Every inbound frame, handshake and request is bounded so a hostile peer only loses its own session. The panels are generated from the built iOS ones; only the Control and Preferences manifests are overridden, and the build fails when their contracts drift.
android-poc becomes android-sample and test-android-poc becomes test-android-sample, both for im.toss.necto.sample. The end-to-end check now covers every shipped panel; the artifact check covers the SDK's permissions, the packaged panels and the debuggable flag. android-connect drops the PoC's injector hook, and docs/verification.md describes the Android checks.
JSON numbers are doubles, which hold every integer only below 2^53. A larger Long was listed as an Int, so the panel parsed it into a rounded number and saving the row unchanged stored that number instead: 9007199254740993 became 9007199254740992. Such a Long is now named Long, which the panel leaves read-only, and a write that reaches the plugin anyway is refused. An integer at 2^53 or beyond is refused too, since the number that arrived may already have been rounded. This follows the iOS fix for containers in toss#21.
A new Android SDK Build & Tests job runs ./gradlew build in SDK/Android, which compiles every module and the sample and runs the JVM and Robolectric tests, then script/test-android-artifacts on the outputs. It runs on ubuntu-24.04 with Temurin JDK 17 and installs the SDK platform and build tools the build and the artifact check expect.
The cherry-picked test called withTestWebsiteDataStore, a helper that lives in android-poc's WebViewTestSupport.swift and was not carried over, so NectoAppTests did not compile. It now passes .nonPersistent() like the other tests in the file.
script/test-android-sample and script/test-android-security pass over USB on a Galaxy S25+ (Android 17), so the note that the USB path was unverified no longer holds.
The limit comes from android-poc. A larger write does not just fail: the inbound frame closes the session, the request fails with PROVIDER_FAILED and Necto reconnects within a second, as checked on a Galaxy S25+.
|
|
||
| private let connections = NectoConnectionCenter() | ||
| private let connections = NectoConnectionCenter( | ||
| androidEndpoint: .fromEnvironment(ProcessInfo.processInfo.environment), |
There was a problem hiding this comment.
I don’t think we can use the PoC setup as-is. Could we manage ADB forwarding and Android connections in Swift within the existing Necto instance, alongside iOS connections, without a separate launcher or environment-based mode?
| events: NectoEventsPlugin = NectoEventsPlugin(), | ||
| network: NectoNetworkPlugin = NectoNetworkPlugin(), | ||
| dataStores: Map<String, DataStore<Preferences>> = emptyMap(), | ||
| ): List<NectoPlugin> = listOfNotNull( |
There was a problem hiding this comment.
Could we keep this PR focused on the SDK and Mac connectivity, and move the plugin implementations to a separate PR?
There was a problem hiding this comment.
Done in f5bb496. Plugins move to a separate PR, which I'll open once this one is merged.
| public enum Connection: String, Sendable, Hashable { | ||
| case usb | ||
| case simulator | ||
| case androidDevice |
There was a problem hiding this comment.
I’ll rename the existing usb and simulator cases to iOSDevice and iOSSimulator in a separate PR.
There was a problem hiding this comment.
Thanks, I've left usb and simulator as they are for your rename.
| val target = resolve(input, requirePath = true) | ||
| if (roots.any { it.directory.canonicalFile == target }) throw invalid("A root cannot be deleted") | ||
| if (!target.exists()) throw NectoBridgeError(NectoBridgeErrorCode.OPERATION_UNAVAILABLE, "'${target.name}' does not exist") | ||
| if (!target.deleteRecursively()) throw IOException("'${target.name}' could not be deleted") |
There was a problem hiding this comment.
Recursive deletion follows nested symlinks and can delete files outside the configured root. Could we delete the links without following their targets?
There was a problem hiding this comment.
Fixed on the plugin branch, with a test. It'll be in the plugin PR.
| if (integer in Int.MIN_VALUE.toDouble()..Int.MAX_VALUE.toDouble()) { | ||
| preferences[intPreferencesKey(key)] = integer.toInt() | ||
| } else { | ||
| preferences[longPreferencesKey(key)] = integer.toLong() |
There was a problem hiding this comment.
Writing 2147483648 to an existing Int key changes its type to Long, causing ClassCastException when the app reads it as Int. Could we reject values outside the existing type’s range?
There was a problem hiding this comment.
Fixed on the plugin branch, with a test. It'll be in the plugin PR.
|
|
||
| override fun close() { | ||
| if (!done.get()) captureRest() | ||
| finish() |
There was a problem hiding this comment.
When reading the response body fails, closing it still records the request as COMPLETED. Could we record it as FAILED with the original error?
There was a problem hiding this comment.
Fixed on the plugin branch, with a test. It'll be in the plugin PR.
| val action = if (count == 0) MotionEvent.ACTION_DOWN else pointerAction(MotionEvent.ACTION_POINTER_DOWN, count) | ||
| dispatch(activity, downTime, action, points.take(count + 1)) | ||
| } | ||
| delay(TAP_HOLD_MS) |
There was a problem hiding this comment.
Could we send ACTION_CANCEL when a tap or swipe is cancelled? Cancelling a tap currently leaves the button pressed and can trigger a long press afterward.
There was a problem hiding this comment.
Fixed on the plugin branch, with a test. It'll be in the plugin PR.
Review asked to keep this change to the SDK and the Mac connection. The Events, Network, Performance, Files, Preferences and UI Control plugins, necto-okhttp and the sample screens that exercise them leave here and return in a change of their own. The sample keeps plugin-sample, so the bridge still has a real round trip, and the Android checks drive it instead of the shipped panels.
Review asked for ADB forwarding and Android connections to live in Swift, in the running Necto, beside the iOS connections. The connection centre now finds adb in the Android SDK, starts its server when none runs, and every probe interval lists USB devices and real emulators, asks their running apps for a Necto socket through run-as, and forwards a private socket to each one it finds. The launcher script, the environment variables and the Android-only mode are gone. Each app on a device gets its own forward and connection, where the old guard allowed one per device. Wireless ADB stays refused, and the names read from the device are checked before they are used.
Each app writes its own endpoint file, so a debuggable app could write a second line naming another package and have Necto treat its socket as that app's. The device now prints a file only when it holds exactly one name, the Mac splits on \n alone with whole-string checks, and a package listed twice is dropped.
From an independent review of the ADB connection: - adb starts a server only for localhost, and refuses for 127.0.0.1 as for a remote host, so after a reboot without Android Studio discovery never began. Necto now asks localhost and checks every exit status. - A failed command read as an empty answer, so a device that went offline for a moment lost its apps. Failures now keep what was found, and a scan counts only when its end marker arrives. - A restarted adb server forgets every forward while apps keep their names. The forwards adb holds are now checked each interval. - The startup sweep removed the forwards of another Necto running beside this one. Socket directories now carry the pid, and only a dead run's are swept: its sockets and then the directory, matched by exact path.
heoblitz
left a comment
heoblitz
left a comment
There was a problem hiding this comment.
Thanks for working through the feedback. I think this will be my final round of review.
| environment: [String: String] = ProcessInfo.processInfo.environment, | ||
| home: String = userHome | ||
| ) -> String? { | ||
| let roots = [environment["ANDROID_HOME"], environment["ANDROID_SDK_ROOT"], home + "/Library/Android/sdk"] |
There was a problem hiding this comment.
It would be helpful to let users configure the Android SDK path in Settings, while keeping automatic detection as the default. Finder-launched apps generally don’t inherit shell environment variables, so custom SDK locations may not be found.
For reference, Flipper’s Android SDK location setting lets users configure androidHome through a file-path field.
There was a problem hiding this comment.
Done in e16534c. Empty keeps auto-detection; a chosen folder is used as is.
| + #"for p in $(ps -A -o NAME= | cut -d: -f1 | sort -u); do case "$k" in *" $p "*) "# | ||
| + #"e=$(run-as "$p" cat no_backup/necto-endpoint 2>/dev/null) || continue; h=${e#necto.}; "# | ||
| + #"[ "$e" = "necto.$h" ] && [ ${#h} -eq 64 ] && [ -z "$(printf %s "$h" | tr -d 0-9a-f)" ] && echo "$p $e";; "# | ||
| + #"esac; done; echo "# + scanDone |
There was a problem hiding this comment.
I’d suggest running pm and ps separately, checking each command’s exit status, and handling parsing/filtering in Swift. The final echo currently masks intermediate failures, so a failed query can be treated as an empty scan.
The existing adb() wrapper already checks exit codes, so separate calls could reuse that logic:
let packagesOutput = try await adb([
"-s", serial, "shell", "pm", "list", "packages", "-3"
])
let processesOutput = try await adb([
"-s", serial, "shell", "ps", "-A", "-o", "NAME="
])
// Parse and filter the outputs in Swift.There was a problem hiding this comment.
Done in 5bebebe. A failed run-as now skips only that app.
There was a problem hiding this comment.
Also checked on a USB device (Galaxy S25+, Android 17): the sample is found and script/test-android-sample passes.
| private fun isValidOrigin(value: String): Boolean { | ||
| if (value == "self") return true | ||
| if (!value.startsWith("https://")) return false | ||
| val host = runCatching { java.net.URI(value).host }.getOrNull() |
There was a problem hiding this comment.
The origin validation rules should be consistent between Android and the Mac host. For example, https://example.com/path passes here but is rejected on the Mac, preventing the panel from loading.
The Swift isValidOrigin() implementation could serve as the reference for the validation rules.
| return "https://" + host + (url.port.map { ":\($0)" } ?? "") | ||
| }.joined(separator: " ") | ||
| let local = "'self' " + external | ||
| contentSecurityPolicy = "default-src \(local); connect-src \(local); " |
There was a problem hiding this comment.
Would it make sense to move CSP enforcement to a separate PR? It changes network access rules for all plugin panels, including existing iOS plugins, so I think we should review its compatibility separately.
For context, the shared NectoPluginWebView configuration installs this handler for every plugin panel.
The panel Content-Security-Policy applies to every plugin panel, including existing iOS plugins, so its compatibility is reviewed in a separate change. The scheme handler, its WebView wiring, the network policy test and the manifest docs return to what main has. The stricter allowedOrigins format check stays.
The Android manifest accepted origins the Mac refuses, such as https://example.com/path, so a plugin could ship a panel that never loads. Kotlin now applies the same rules as Swift's isValidOrigin, and ModelTests runs the same accept and reject cases.
The scan ran in one shell whose final echo masked a failed pm or ps, so a failed query could pass for an empty scan. pm and ps now run on their own through adb(), which checks each exit status, and Swift parses and intersects their output. Each app's endpoint file is read with its own run-as: a failure skips that app, and get-state then confirms the device still answers, so a device that left is not read as one without apps. The run-as calls run side by side, about 0.2 s for 26 apps on a phone against 1.5 s one after another.
An app started from the Finder does not inherit the shell's variables, so an SDK outside the usual places was never found. Settings > General now has an Android SDK field: empty keeps automatic detection, and a chosen folder is the only place looked at. The hint says whether adb was found. Changing it moves Android discovery to the new adb without a relaunch; every adb talks to the same server, so forwards stay, and a folder without adb lets the Android apps go.
|
Thanks for the detailed reviews across every round! Thanks to them, I learned a lot about keeping a desktop app consistent across iOS and Android. |
Summary
Refs #17. Adds the Android SDK under
SDK/Android/, following the agreed direction:android-poc's panel CSP hardening along.necto-core(JVM: protocol, transport, runtime),necto-androidandsample, under theim.toss.nectopackage and Maven group.Checkgets anAndroid SDK Build & Testsjob.The wire protocol is unchanged.
One difference from iOS: inbound frames keep
android-poc's 64 KiB limit, while iOS accepts 32 MiB. Should I raise it to match?Moving
Package.swiftandExampleApp/underSDK/iOS/is left out to keep this PR Android-only.How Necto finds an Android app
adb devicesevery 2 s (iOS'sprobeInterval), USB and real emulators onlyrun-asrun-ason all 378 installed apps took 5.7 s, so Necto asks running third-party apps only (~0.75 s)./proc/net/unixis blocked by SELinux.Review guide
About 3.2k lines of code and 1.2k of tests, plus the Gradle wrapper. Commit by commit is easiest.
NectoADB.swift,NectoConnectionCenter.swift,NectoAndroidListener.kt, and thenecto-coretransport and runtime.sample/and the Gradle wrapper.If it's too much at once, I can split it into three PRs (core → android + Mac → sample).
Changelog
Add an Android SDK that connects to Necto over ADB.
Test Plan
cd SDK/Android && ./gradlew build: the core tests pass.script/test-android-artifacts: the SDK declares no permissions,necto-corecarries no panels, and only the debug build is debuggable.script/test: the Swift, Mac app, web and release checks pass locally (Xcode 27.0).script/test-android-sampleandscript/test-android-security: pass on an Android 17 emulator and over USB on a Galaxy S25+ (Android 17). Necto connected 3.7 s after it started.Checkin my fork, before the review changes: all six jobs pass. The first attempt hung inswift test --package-path NectoMac. A stack sample from a local repro shows the socket-connect deadlock that 440bdc0 fixed onmain. That commit isn't infeature/android-supportyet, so syncing the branch withmainshould fix it.Not verified: Android E2E isn't in CI yet, and only one physical device was tested.
Follow-ups