diff --git a/.env.example b/.env.example index 5181984..7011e22 100644 --- a/.env.example +++ b/.env.example @@ -8,7 +8,7 @@ COOKIE_SECURE=false ALLOWED_ORIGINS=http://mg4pi.local:8787 UPDATE_STATUS_PATH=/var/lib/pi-car-companion/update-status.json VERSION_FILE=/opt/pi-car-companion/current/REVISION -# Disabled by default. When enabled, ADB_SERIAL must identify exactly one authorized head unit. +# Disabled by default. Leave ADB_SERIAL empty to auto-select only when exactly one device is attached. ADB_ENABLED=false ADB_PATH=adb ADB_SERIAL= diff --git a/PLAN.md b/PLAN.md index e13471c..e5b81b3 100644 --- a/PLAN.md +++ b/PLAN.md @@ -227,8 +227,9 @@ reference, not proof for other model years, trims, regions, or software builds. - Car UX restrictions are enforced while moving. Version 1 is parked-use software unless a later native activity is explicitly designed, declared, and validated as distraction optimized. -- The Pi is the only intended ADB host. A future bridge must target one configured - device, use argument-array process execution, enforce timeouts and output bounds, +- The Pi is the only intended ADB host. The bridge targets either one configured + serial or exactly one automatically discovered device, refuses ambiguous multi-device + connections, uses argument-array process execution, enforces timeouts and output bounds, expose unavailable/unauthorized/offline states, and register every operation in a typed server-side allowlist. No API accepts a shell string, arbitrary ADB arguments, a package/component supplied by a client, or an unrestricted file path. diff --git a/README.md b/README.md index 8c4ab98..30108ec 100644 --- a/README.md +++ b/README.md @@ -151,15 +151,19 @@ Important values: - `UPDATE_STATUS_PATH`: update state shared with the systemd update unit - `VERSION_FILE`: revision metadata for the active atomic release - `ADB_ENABLED`: enables the read-only head-unit collector; defaults to `false` -- `ADB_SERIAL`: exact device serial used for every ADB command when enabled +- `ADB_SERIAL`: optional exact serial; when empty, discovery requires exactly one attached device - `ADB_PATH` and `ADB_TIMEOUT_MS`: executable path and per-command timeout ### Read-only ADB setup -ADB integration is optional and disabled by default. Every command is pinned to one -configured serial, uses a fixed server-side argument list, and has a timeout and 16 KiB -output limit. It currently reads connection state, selected `getprop` identity fields, -and `wm size`/`wm density` only. +ADB integration is optional and disabled by default. It starts the ADB server when the +companion service boots. With no configured serial it automatically selects exactly one +attached device and refuses ambiguous multi-device connections; after selection every +command is serial-pinned. Commands use fixed server-side argument lists with a timeout +and 16 KiB output limit. The ADB authorization key persists in the companion data +directory, so an already-authorized car reconnects after Pi reboot or USB reattachment. +The collector currently reads connection state, selected `getprop` identity fields, and +`wm size`/`wm density` only. On a deployed Pi, rerun `install.sh` once so `android-tools-adb` is present and the service account receives the standard `plugdev` device-access group. Connect the head @@ -174,7 +178,8 @@ Accept the authorization prompt on the parked infotainment screen. Set these val ```text ADB_ENABLED=true -ADB_SERIAL= +# Optional when only the MG4 is attached: +ADB_SERIAL= ``` If the device is disconnected, offline, unauthorized, partially readable, or ADB is diff --git a/install.sh b/install.sh index 85583aa..f7e1215 100755 --- a/install.sh +++ b/install.sh @@ -129,6 +129,10 @@ COOKIE_SECURE=false ALLOWED_ORIGINS=http://mg4pi.local:8787 UPDATE_STATUS_PATH=${DATA_DIR}/update-status.json VERSION_FILE=${CURRENT}/REVISION +ADB_ENABLED=false +ADB_PATH=adb +ADB_SERIAL= +ADB_TIMEOUT_MS=3000 EOF fi chown root:"${SERVICE_USER}" "${CONFIG_DIR}/companion.env" diff --git a/server/src/adb.ts b/server/src/adb.ts index 6c130b2..038ba2e 100644 --- a/server/src/adb.ts +++ b/server/src/adb.ts @@ -26,7 +26,7 @@ export type AdbExecutor = ( options: ExecuteOptions ) => Promise<{ stdout: string; stderr: string }>; -export type AdbConnectionState = "disabled" | "unavailable" | "offline" | "unauthorized"; +export type AdbConnectionState = "disabled" | "unavailable" | "offline" | "unauthorized" | "ambiguous"; export type AdbDeviceSnapshot = { state: "connected"; @@ -111,15 +111,42 @@ export async function collectAdbStatus( if (!config.enabled) { return { available: false, state: "disabled", reason: "ADB integration is not configured" }; } - if (!config.serial) { - return { available: false, state: "unavailable", reason: "ADB is enabled without a configured device serial" }; + + const options = { timeout: config.timeoutMs, maxBuffer: MAX_ADB_OUTPUT_BYTES }; + let serial = config.serial; + if (!serial) { + try { + const output = (await execute(config.executable, ["devices"], options)).stdout; + const devices = output + .split(/\r?\n/) + .slice(1) + .map((line) => line.trim().split(/\s+/, 3)) + .filter((parts): parts is [string, string, ...string[]] => Boolean(parts[0] && parts[1])); + if (devices.length === 0) { + return { available: false, state: "offline", reason: "No ADB infotainment device is connected" }; + } + if (devices.length > 1) { + return { + available: false, + state: "ambiguous", + reason: "Multiple ADB devices are connected; configure ADB_SERIAL to select the infotainment unit" + }; + } + const [discoveredSerial, discoveredState] = devices[0]!; + if (discoveredState === "unauthorized") { + return { available: false, state: "unauthorized", reason: "Authorize this Pi on the infotainment screen" }; + } + if (discoveredState !== "device") { + return { available: false, state: "offline", reason: "The discovered infotainment device is offline" }; + } + serial = discoveredSerial; + } catch (error) { + return connectionFailure(error); + } } const run = async (arguments_: readonly string[]) => - execute(config.executable, ["-s", config.serial!, ...arguments_], { - timeout: config.timeoutMs, - maxBuffer: MAX_ADB_OUTPUT_BYTES - }); + execute(config.executable, ["-s", serial, ...arguments_], options); try { const state = (await run(["get-state"])).stdout.trim().toLowerCase(); diff --git a/server/src/app.ts b/server/src/app.ts index b146ee9..00c0f25 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -19,6 +19,7 @@ import { } from "./security.js"; import { collectSystemStatus } from "./status.js"; import { readUpdateStatus, triggerSystemUpdate } from "./update.js"; +import { collectAdbStatus } from "./adb.js"; import "./types.js"; const usernameSchema = z @@ -98,6 +99,7 @@ export async function buildApp(config: AppConfig): Promise { }); const database = openDatabase(config.databasePath); const jobRunner = new JobRunner(database, undefined, config.adb); + if (config.adb.enabled) void collectAdbStatus(config.adb); app.decorate("database", database); app.decorate("jobRunner", jobRunner); diff --git a/server/src/config.ts b/server/src/config.ts index 1370f31..81f8e02 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -50,10 +50,6 @@ export function loadConfig(environment: NodeJS.ProcessEnv = process.env): AppCon } const adbSerial = parsed.ADB_SERIAL.trim() || null; - if (parsed.ADB_ENABLED && !adbSerial) { - throw new Error("ADB_SERIAL is required when ADB_ENABLED=true"); - } - return { nodeEnv: parsed.NODE_ENV, host: parsed.HOST, diff --git a/server/tests/adb.test.ts b/server/tests/adb.test.ts index 7edc57f..e98d624 100644 --- a/server/tests/adb.test.ts +++ b/server/tests/adb.test.ts @@ -80,6 +80,42 @@ describe("read-only ADB collector", () => { }); }); + it("automatically discovers exactly one device and refuses ambiguous connections", async () => { + const calls: string[][] = []; + const execute: AdbExecutor = async (_executable, arguments_) => { + calls.push([...arguments_]); + if (arguments_[0] === "devices") { + return { stdout: "List of devices attached\nAUTO-MG4\tdevice product:saic model:autus\n", stderr: "" }; + } + if (arguments_.at(-1) === "get-state") return { stdout: "device\n", stderr: "" }; + return { stdout: "value\n", stderr: "" }; + }; + const status = await collectAdbStatus({ ...enabledConfig, serial: null }, execute); + expect(status).toMatchObject({ available: true, value: { state: "connected" } }); + expect(calls[0]).toEqual(["devices"]); + for (const arguments_ of calls.slice(1)) expect(arguments_.slice(0, 2)).toEqual(["-s", "AUTO-MG4"]); + + const multiple: AdbExecutor = async () => ({ + stdout: "List of devices attached\nMG4\tdevice\nPHONE\tdevice\n", + stderr: "" + }); + await expect(collectAdbStatus({ ...enabledConfig, serial: null }, multiple)).resolves.toMatchObject({ + available: false, + state: "ambiguous" + }); + }); + + it("reports an automatically discovered device awaiting authorization", async () => { + const execute: AdbExecutor = async () => ({ + stdout: "List of devices attached\nAUTO-MG4\tunauthorized\n", + stderr: "" + }); + await expect(collectAdbStatus({ ...enabledConfig, serial: null }, execute)).resolves.toMatchObject({ + available: false, + state: "unauthorized" + }); + }); + it("keeps individual fields unavailable when a connected device only partially responds", async () => { const execute: AdbExecutor = async (_executable, arguments_) => { if (arguments_.at(-1) === "get-state") return { stdout: "device\n", stderr: "" }; diff --git a/server/tests/config.test.ts b/server/tests/config.test.ts index 08ef0af..7909545 100644 --- a/server/tests/config.test.ts +++ b/server/tests/config.test.ts @@ -11,8 +11,8 @@ describe("ADB configuration", () => { }); }); - it("requires a constrained serial when enabled", () => { - expect(() => loadConfig({ NODE_ENV: "test", ADB_ENABLED: "true" })).toThrow("ADB_SERIAL is required"); + it("supports automatic discovery or a constrained pinned serial when enabled", () => { + expect(loadConfig({ NODE_ENV: "test", ADB_ENABLED: "true" }).adb).toMatchObject({ enabled: true, serial: null }); expect(() => loadConfig({ NODE_ENV: "test", ADB_ENABLED: "true", ADB_SERIAL: "serial; shell command" }) ).toThrow(); diff --git a/systemd/pi-car-companion.service b/systemd/pi-car-companion.service index 978883b..fb5fee6 100644 --- a/systemd/pi-car-companion.service +++ b/systemd/pi-car-companion.service @@ -9,6 +9,7 @@ User=pi-companion Group=pi-companion WorkingDirectory=/opt/pi-car-companion/current EnvironmentFile=/etc/pi-car-companion/companion.env +Environment=HOME=/var/lib/pi-car-companion ExecStart=/usr/bin/node /opt/pi-car-companion/current/server/dist/index.js Restart=on-failure RestartSec=5s diff --git a/web/src/App.tsx b/web/src/App.tsx index 39add77..377e300 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -328,11 +328,13 @@ function StatusContent({ status }: { status: SystemStatus }) { : "Connected" : status.headUnit.state === "unauthorized" ? "Authorization required" - : status.headUnit.state === "disabled" - ? "Not configured" - : status.headUnit.state === "offline" - ? "Offline" - : "Unavailable"; + : status.headUnit.state === "ambiguous" + ? "Multiple devices" + : status.headUnit.state === "disabled" + ? "Not configured" + : status.headUnit.state === "offline" + ? "Offline" + : "Unavailable"; return (
@@ -359,7 +361,7 @@ function StatusContent({ status }: { status: SystemStatus }) {
Operating system{status.operatingSystem}
Processor{status.cpu.model}
Wi-Fi detail{status.network.wifi.available ? status.network.wifi.value : status.network.wifi.reason}
-
MG4 head unit{headUnitLabel}
+
ADB connection{headUnitLabel}
); diff --git a/web/src/api.ts b/web/src/api.ts index 0d749bc..723a7f0 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -31,7 +31,7 @@ export type HeadUnitStatus = } | { available: false; - state: "disabled" | "unavailable" | "offline" | "unauthorized"; + state: "disabled" | "unavailable" | "offline" | "unauthorized" | "ambiguous"; reason: string; };