fix: contain managed ADB transport to its lease target

[thymikee]

Summary

Pin managed Android ADB commands and background spawners to the lease's serial and private localhost server. Foreign serials, ports, global selectors, and server-mode overrides now fail before native dispatch; unqualified commands cannot inherit another target.

ADR 0021 prerequisite for reviewed managed automation, based on #2308. Child sockets are explicitly cleared and server administration is denied as defensive hardening. Ordinary and provider transports retain their existing behavior. Scope: 5 files, 268 gross lines.

Validation

Tested commit: 65bcfb1696037872ff2159a19097f18153ea2731.

The exact-head pnpm check:affected --run passed, including 3,984 tests across 521 files, build, typecheck, lint, layering and Fallow.

Independent adversarial review found no concrete issues. Native ADB fixtures cover execution, background spawning, preconstructed executors, concurrent private servers, inherited environment, and parser escape attempts; regression tests were observed red before the fixes.

Managed request admission and publication remain unimplemented in this layer. Live verification requires that later path and an allocator-owned emulator; fixture evidence does not establish merge readiness. GitHub remains authoritative for native, provider, and coverage lanes.

Docs and skills are unchanged because this is an internal transport fix.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.48 MB	4.48 MB	+1.0 kB
Package (unpacked)	4.48 MB	4.48 MB	+1.0 kB
Package (download)	1.33 MB	1.33 MB	+325 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.4 ms	26.4 ms	-1.0 ms
CLI --help	83.8 ms	79.5 ms	-4.4 ms

[thymikee]

Code review

The containment model reads well: withServerPort is a good last-resort choke point, and routing every managed call through -s <lease serial> closes the stripAdbSerialArgs smuggling path (adb -s DEVICE -s OTHER shell id now dies at the duplicate-serial check). Two things I'd want fixed before this lands.

1. ADB_SERVER_SOCKET is not pinned, and the test added for it asserts nothing — src/platform-runtime-android-adb-host.ts:157

adbInvocation pins ANDROID_ADB_SERVER_PORT and ANDROID_ADB_SERVER_ADDRESS, but the child env starts from ...environment (line 154), so an ADB_SERVER_SOCKET present in the daemon's own environment is inherited verbatim.

adb resolves its server socket in this order: -L, then ADB_SERVER_SOCKET, then a tcp:<ANDROID_ADB_SERVER_ADDRESS>:<ANDROID_ADB_SERVER_PORT> built from the other two. Managed calls pass -P, never -L. So with ADB_SERVER_SOCKET=tcp:foreign.host:5037 exported into the daemon, every managed command either goes to the foreign server or aborts with -L is incompatible with -H or -P. Both defeat the pinning this PR is for.

The test at src/platform-runtime-android-adb-host.test.ts:85 looks like it covers this, but it cannot: the stub adb at line 53 only echoes args, ANDROID_ADB_SERVER_PORT and ANDROID_ADB_SERVER_ADDRESS, and serialWithWrongEnvironment is typed { args, port }. assert.deepEqual(serialWithWrongEnvironment, serial) passes whatever ADB_SERVER_SOCKET does.

Fix: delete ADB_SERVER_SOCKET from the child env next to the two pinned variables, and make the stub report it so the assertion has teeth.

2. Server-lifecycle verbs still reach the allocator's private server — src/platform-runtime-android-adb-host.ts:182

The guard rejects nodaemon, server and fork-server, but kill-server, start-server, connect and disconnect are none of those and do not start with -, so the loop breaks on them and they pass through with the lease's -P attached:

runAndroidHostAdb(['kill-server'])  ->  adb -P 15037 -s emulator-15037 kill-server

That kills the allocator-owned server and every other lease on it. ['disconnect'] with no argument drops every network device from the same server. Before this change both went to the developer's own 5037 server, so this is a new blast radius rather than a pre-existing one.

Since the guard already enumerates server verbs and the fixtures treat arbitrary argv as hostile, these belong in the same rejection list — or, better, invert it to an allowlist of the verbs managed code actually issues.

Checked and fine

• adb -s <serial> devices / version / emu kill: -s is inert for globals, so the prefixing does not change results.
• ANDROID_SERIAL is inherited but neutralized, because -s is now always present on every managed argv.
• Pre-scope executors: scopedServerPort reads the ambient store at call time and falls back to the constructor port outside a scope, so no silent downgrade to 5037.
• -e / -d args in am instrument and logcat payloads all sit past the first non-flag argument, so the stricter loop does not reach them.
• runCmdBackground bypasses the command-executor override, but the Android app-log path goes through resolveScopedAndroidAdbBackgroundTransport -> the scoped spawner, so it stays contained.

[thymikee]

Sentinel review at e5f3c7c: Independent sentinel review found no confirmed source defect in the supported managed route. Correction to the earlier review: explicit -P causes ADB to ignore ADB_SERVER_SOCKET (AOSP client/commandline.cpp, https://android.googlesource.com/platform/packages/modules/adb/+/refs/heads/main/client/commandline.cpp). The socket-escape claim is therefore not valid here. Global server verbs are not serial-scoped, but no supported managed operation at this head supplies arbitrary host-ADB argv; lifecycle paths are withheld/delegated, so additional verb filtering is hardening rather than a demonstrated shipped escape. Readiness remains blocked: iOS bridge preparation timed out probing the SDK (xcrun, 10s), prerequisite stack order remains, and the PR explicitly lacks live allocator-owned emulator evidence. Fixtures are not live acceptance.

[thymikee]

Addressed both requests in 65bcfb1696037872ff2159a19097f18153ea2731:

• Explicitly remove ADB_SERVER_SOCKET from the child environment. The native fixture reports its value and checks inherited and per-command overrides, while preserving the parent environment.
• Reject server administration, including kill-server, start-server, connect, disconnect, reconnect/attach/detach/pairing, and commands behind ADB's wait-for-* prefix.

As the sentinel correction notes, explicit -P already overrides ADB_SERVER_SOCKET; this is defensive hardening, not evidence of an existing supported-route escape. The added assertions were observed failing with the corresponding protections removed. All 47 focused transport tests pass, and independent review found no remaining findings.

The exact-head pnpm check:affected --run passed, including 3,984 tests across 521 files. Restacked onto published #2308 1df42d51f50e7dfb3162f113f74a76829c9b8272; range-diff confirms both ADB commits are unchanged. GitHub CI is pending; not waiting for it as requested.

Live allocator-owned emulator verification still depends on the later managed admission/publication path. No merge-readiness claim.

The latest-main restack is rooted at 80997b6bf1dda30ca57d74dc42036f6b447e16ad. Both ADB patches remain unchanged.

[thymikee]

Sentinel at 65bcfb1: no actionable source findings after coordinator and independent Terra/high review. Managed ADB pins the lease serial/private port and child server environment, rejects foreign selectors/resolvers and blocks server administration; the negative cases would fail against the base implementation. Exact-head CI is green. Still evidence-pending, not merge-ready: allocator-owned managed-emulator proof through the activated admission/publication route is absent. Provide live evidence that operations stay on the leased transport and cannot escape to the host default server or another device. The prerequisite #2308 also has failing iOS CI. No readiness label, fixes or merge.

[thymikee]

Fresh review pass at 65bcfb1696037872ff2159a19097f18153ea2731:

• The original socket-environment and server-administration requests are already implemented and covered by the mapped regression evidence above. The socket-escape claim was corrected by the earlier sentinel review.
• The new sentinel comment reports no actionable source defect. All current-head required checks, including Integration, Coverage, and iOS smoke, are green. No unresolved inline review threads remain.
• The requested live allocator-owned emulator acceptance is still outstanding. This layer provides containment mechanics; its lease-admission and reachability constructors have no activated production caller. Running another native fixture would not establish the requested end-to-end proof. Acceptance must run through the later activated admission/publication route using a verified allocator-owned emulator, checking that operations cannot reach the host-default server or a foreign target.

Independent fresh read-only review agrees: no actionable source findings; the activated allocation/admission/publication path is the remaining live-proof prerequisite.

No code changes, new capabilities, or readiness claim. The existing exact-head local gate evidence remains applicable; no redundant push or gate rerun was needed. #2312 will be reviewed next on this unchanged published parent. Nothing merged.