diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index 8ba8895b3..d05a0aa8d 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -233,7 +233,6 @@ test('connect proxy writes normal remote state with generated non-secret profile assert.equal(state.leaseId, undefined); assert.deepEqual(state.daemon, { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.match(state.remoteConfigPath, /remote-connections\/generated\/proxy-[a-f0-9]{16}\.json$/); @@ -277,7 +276,6 @@ test('connect daemon-base-url shortcut uses proxy profile for direct proxy URLs' assert.match(state.clientId ?? '', /^[a-f0-9]{16}$/); assert.deepEqual(state.daemon, { baseUrl: 'http://127.0.0.1:4310/agent-device', - authToken: 'proxy-secret', transport: 'http', }); assert.equal(state.leaseId, undefined); @@ -2269,7 +2267,6 @@ test('disconnect releases proxy lease with provider client and device metadata', leaseId: 'abc123abc123abc1', daemon: { baseUrl: 'http://proxy.example.test/agent-device', - authToken: 'proxy-secret', }, leaseBackend: 'ios-instance', leaseProvider: 'proxy', @@ -2290,6 +2287,10 @@ test('disconnect releases proxy lease with provider client and device metadata', version: false, stateDir, shutdown: true, + // Not persisted on connection state (ADR 0007): the caller supplies + // it per-command via flag/env/CLI-session, mirroring how the CLI + // dispatcher resolves it before invoking this handler. + daemonAuthToken: 'test-not-a-real-token', }, client: createTestClient({ release: async (request) => { @@ -2306,7 +2307,7 @@ test('disconnect releases proxy lease with provider client and device metadata', assert.equal(releaseRequest?.leaseId, 'abc123abc123abc1'); assert.equal(releaseRequest?.leaseBackend, 'ios-instance'); assert.equal(releaseRequest?.daemonBaseUrl, 'http://proxy.example.test/agent-device'); - assert.equal(releaseRequest?.daemonAuthToken, 'proxy-secret'); + assert.equal(releaseRequest?.daemonAuthToken, 'test-not-a-real-token'); assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' }), null); fs.rmSync(tempRoot, { recursive: true, force: true }); }); diff --git a/src/bin.ts b/src/bin.ts index 18b0a2006..4afd45ebb 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -1,3 +1,5 @@ +import { normalizeCliCommandAlias } from './commands/cli-command-aliases.ts'; + const argv = process.argv.slice(2); declare const __AGENT_DEVICE_VERSION__: string; @@ -54,7 +56,7 @@ function runHelpFastPath(argv: string[]): boolean { process.stdout.write(`${buildUsageText()}\n`); return; } - const commandHelp = buildCommandUsageText(normalizeHelpTarget(helpTarget)); + const commandHelp = buildCommandUsageText(normalizeCliCommandAlias(helpTarget)); if (commandHelp) { process.stdout.write(commandHelp); return; @@ -97,12 +99,6 @@ function resolveTrailingHelpTarget( return isHelpFlag(helpArg) ? command : undefined; } -function normalizeHelpTarget(command: string): string { - if (command === 'long-press') return 'longpress'; - if (command === 'metrics') return 'perf'; - return command; -} - function isHelpCommand(command: string | undefined): boolean { return command === 'help'; } diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index ad41010ed..15dd78d63 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -483,6 +483,9 @@ export async function stopReactDevtoolsCleanup(options: { export async function releaseRemoteConnectionLease( client: AgentDeviceClient, state: RemoteConnectionState, + // The daemon bearer token is never persisted on `state` (ADR 0007); callers + // pass the token already resolved via the flag/env/CLI-session chain. + daemonAuthToken?: string, ): Promise<{ released: boolean; provider?: CloudProviderSessionResult }> { if (!state.leaseId) return { released: false }; const result = await client.leases.release({ @@ -491,7 +494,7 @@ export async function releaseRemoteConnectionLease( leaseId: state.leaseId, leaseBackend: state.leaseBackend, daemonBaseUrl: state.daemon?.baseUrl, - daemonAuthToken: state.daemon?.authToken, + daemonAuthToken, daemonTransport: state.daemon?.transport, daemonServerMode: state.daemon?.serverMode, leaseProvider: state.leaseProvider, @@ -504,10 +507,11 @@ export async function releaseRemoteConnectionLease( export async function releasePreviousLease( client: AgentDeviceClient, previous: RemoteConnectionState, + daemonAuthToken?: string, ): Promise { if (!previous.leaseId) return; try { - await releaseRemoteConnectionLease(client, previous); + await releaseRemoteConnectionLease(client, previous, daemonAuthToken); } catch { // Reconnect must succeed even if the old lease was already released. } diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index 91989a9a7..f3c8e7d10 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -244,7 +244,7 @@ async function cleanupForcedPreviousConnection( if (!previous || !flags.force) return; await stopMetroCleanup(previous.metro); await stopReactDevtoolsCleanup({ stateDir, state: previous }); - await releasePreviousLease(client, previous); + await releasePreviousLease(client, previous, flags.daemonAuthToken); } function readRemoteConfigConnectionMetadata( @@ -286,7 +286,7 @@ export const disconnectCommand: ClientCommandHandler = async ({ flags, client }) let released = false; if (state.leaseId) { try { - const release = await releaseRemoteConnectionLease(client, state); + const release = await releaseRemoteConnectionLease(client, state, flags.daemonAuthToken); released = release.released; providerData ??= release.provider; } catch { diff --git a/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts b/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts new file mode 100644 index 000000000..1e164223d --- /dev/null +++ b/src/cli/parser/__tests__/cli-help-alias-fast-path.test.ts @@ -0,0 +1,66 @@ +// Pins the exact composition `bin.ts`'s `--help` fast path relies on: +// `buildCommandUsageText(normalizeCliCommandAlias(helpTarget))`. Before this +// fix, `bin.ts` used its own hand-written two-entry table instead of this +// composition, so `tap`, `launch`, and `relaunch` silently missed the fast +// path and fell through to a full CLI bootstrap just to print static help +// text. `bin.ts` runs unguarded top-level dispatch on import (and is +// deliberately excluded from coverage — see vitest.config.ts), so it cannot +// be imported directly in a test; these tests instead pin the registry +// composition it calls. That makes them a real regression pin for a *future* +// alias missing help text (test 2 is durable for that), but not a substitute +// for the manual proof, run outside this suite, that bin.ts itself calls +// this composition (see the plan's execution report for the red/green +// evidence: with the stale table, `tap --help` loads `src/cli.ts`; with this +// fix, it does not). +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { buildCommandUsageText } from '../cli-help.ts'; +import { + cliAliasesForCommand, + normalizeCliCommandAlias, +} from '../../../commands/cli-command-aliases.ts'; +import { listCliCommandNames } from '../../../command-catalog.ts'; + +test('alias help output matches its canonical command', () => { + const cases: ReadonlyArray = [ + ['tap', 'press'], + ['launch', 'open'], + ['relaunch', 'open'], + ['long-press', 'longpress'], + ['metrics', 'perf'], + ]; + for (const [alias, canonical] of cases) { + const aliasHelp = buildCommandUsageText(normalizeCliCommandAlias(alias)); + const canonicalHelp = buildCommandUsageText(canonical); + assert.notEqual(aliasHelp, null, `expected help text for alias "${alias}"`); + assert.equal( + aliasHelp, + canonicalHelp, + `expected "${alias} --help" to be byte-identical to "${canonical} --help"`, + ); + } +}); + +test('every CLI alias resolves to a command with help text', () => { + // Derive the alias list from the registry itself (via the canonical + // commands it targets) rather than hard-coding the five current alias + // names — a hard-coded list would silently stop covering a future sixth + // alias, reintroducing exactly the drift this test exists to catch. + const aliases = listCliCommandNames().flatMap((command) => + cliAliasesForCommand(command).map((entry) => entry.alias), + ); + assert.ok(aliases.length > 0, 'expected at least one alias to exercise this test'); + for (const alias of aliases) { + const help = buildCommandUsageText(normalizeCliCommandAlias(alias)); + assert.notEqual(help, null, `expected buildCommandUsageText to resolve alias "${alias}"`); + } +}); + +test('rotate still has no fast-path help', () => { + // `rotate` is not in the alias registry, so it must fall through to the + // slow path (`src/cli/parser/args.ts`'s `normalizeCommandAlias`), which is + // where the "renamed to orientation" migration error is raised. The fast + // path must never special-case `rotate` itself. + const help = buildCommandUsageText(normalizeCliCommandAlias('rotate')); + assert.equal(help, null); +}); diff --git a/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts b/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts index 250c418fc..680057415 100644 --- a/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts +++ b/src/daemon/handlers/__tests__/interaction-ios-tap-outcome.test.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { AppError } from '@agent-device/kernel/errors'; +import { buildSnapshotPresentationKey } from '@agent-device/kernel/snapshot'; import { handleInteractionCommands } from '../interaction.ts'; import { handleSnapshotCommands } from '../snapshot.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; @@ -234,6 +235,52 @@ test('a changed capture with a different presentation keeps the tap failure', as expect(sessionStore.get(sessionName)?.actions).toHaveLength(0); }); +test('corroborates a tap when the request carries no flags and the baseline used a non-default scope', async () => { + const sessionName = 'ios-no-flags-tap-corroboration'; + const sessionStore = makeSessionStore(); + const baseline = snapshot(profileNodes); + baseline.presentationKey = buildSnapshotPresentationKey({ depth: 2, raw: false }); + sessionStore.set( + sessionName, + makeIosSession(sessionName, { + appBundleId: 'com.example.app', + snapshot: baseline, + }), + ); + mockDispatch.mockImplementation(async (_device, command) => { + if (command === 'press') { + throw new AppError( + 'XCTEST_RECORDED_FAILURE', + 'XCTest recorded a failure while executing tap; the action may not have been performed.', + ); + } + if (command === 'snapshot') return snapshotPayload(imageViewerNodes); + return {}; + }); + + // Deliberately built without a `flags` key at all (not `flags: {}`) — this + // mirrors the real production paths (batch steps with no flags, JSON-RPC + // requests that omit the key) that hide the bug this test pins. + const response = await handleInteractionCommands({ + req: { + token: 'test', + session: sessionName, + command: 'click', + positionals: ['id="unfollow"'], + }, + sessionName, + sessionStore, + contextFromFlags, + }); + + expect(response?.ok).toBe(true); + if (response?.ok) { + expect(response.data?.warning).toMatch(/post-action accessibility capture changed/); + expect(response.data?.selector).toBe('id="unfollow"'); + } + expect(sessionStore.get(sessionName)?.actions).toHaveLength(1); +}); + test('a changed capture against a stale baseline keeps the tap failure', async () => { const sessionName = 'ios-stale-baseline-tap-corroboration'; const sessionStore = makeSessionStore(); diff --git a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts index 7c0a9f91b..7bca26a84 100644 --- a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts +++ b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts @@ -69,6 +69,9 @@ import { stopAndroidSnapshotHelperSessionForDevice } from '../../../platforms/an import { stopIosRunnerSession } from '../../../platforms/apple/core/runner/runner-client.ts'; import { WEB_DESKTOP_DEVICE } from '../../../__tests__/test-utils/index.ts'; import { setActiveProviderDeviceRuntimes } from '../../../provider-device-runtime.ts'; +import { acquireAdvisoryDeviceClaim } from '../../device-claims.ts'; +import { inspectDeviceClaims } from '../../device-claim-inspection.ts'; +import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../../../utils/diagnostics.ts'; const mockShutdownSimulator = vi.mocked(shutdownSimulator); const mockRunCmd = vi.mocked(runCmd); @@ -1077,6 +1080,161 @@ test('targeted close preserves the platform-close AppError and still runs later expect(sessionStore.get(sessionName)).toBeUndefined(); }); +// #1478-adjacent (device-claim retention observability): a failed platform close deliberately +// keeps the advisory device claim (handing an unconfirmed device to the next session would be +// worse), but the session record is still deleted on the very next line. Before this pair of +// tests, nothing said so — the claim just quietly named a session `session list` no longer +// reported. These two tests pin both branches of that decision so a future refactor cannot +// silently invert either one. +test('a failed platform close retains the device claim and reports it', async () => { + const claimsRoot = mkdtempForTestSync('agent-device-session-close-claim-retained-'); + const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR; + process.env.AGENT_DEVICE_CLAIMS_DIR = claimsRoot; + try { + const sessionStore = makeSessionStore(); + const sessionName = 'targeted-close-claim-retained-session'; + const device = { + platform: 'apple' as const, + id: 'sim-udid-close-claim-retained', + name: 'iPhone 15', + kind: 'simulator' as const, + booted: true, + }; + const acquired = await acquireAdvisoryDeviceClaim({ + device, + session: sessionName, + workspace: process.cwd(), + stateDir: sessionStore.resolveDaemonStateDir(), + }); + if (!acquired.ownership) { + throw new Error('expected the test session to acquire a device claim'); + } + const session = { + ...makeSession(sessionName, device), + // Recording defeats runner retention so this mirrors the platform-close-error test above + // rather than exercising a different code path. + recording: { outPath: '/tmp/recording.mp4' }, + deviceClaim: acquired.ownership, + } as unknown as SessionState; + sessionStore.set(sessionName, session); + + const platformCloseError = new AppError('DEVICE_UNAVAILABLE', 'platform close failed', { + reason: 'device_disconnected', + hint: 'Reconnect the device and retry close.', + }); + mockDispatchCommand.mockRejectedValueOnce(platformCloseError); + + const diagnosticsLogPath = path.join(claimsRoot, 'diagnostics.ndjson'); + const thrown = await withDiagnosticsScope( + { session: sessionName, command: 'close', logPath: diagnosticsLogPath }, + async () => { + let caught: unknown; + try { + await handleSessionCommands({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: ['com.example.app'], + flags: {}, + }, + sessionName, + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + } catch (error) { + caught = error; + } + flushDiagnosticsToSessionFile({ force: true }); + return caught; + }, + ); + + expect(thrown).toBe(platformCloseError); + // The session is still deleted (deliberate, pre-existing behavior) even though the claim + // could not be confirmed released. + expect(sessionStore.get(sessionName)).toBeUndefined(); + + // The claim itself was NOT cleared: it is still live, still naming the deleted session. + const claimState = inspectDeviceClaims({ serial: device.id })[0]; + expect(claimState?.classification).toBe('live'); + expect(claimState?.claim?.session).toBe(sessionName); + + // A warn diagnostic names the retained claim's device key and owning session so the retention + // is observable instead of silent. + const rows = fs + .readFileSync(diagnosticsLogPath, 'utf8') + .trim() + .split('\n') + .map((line) => JSON.parse(line)); + expect(rows).toContainEqual( + expect.objectContaining({ + level: 'warn', + phase: 'device_claim_close_effects_unconfirmed', + data: { deviceKey: acquired.ownership.deviceKey, session: sessionName }, + }), + ); + } finally { + if (previousClaimsDir === undefined) delete process.env.AGENT_DEVICE_CLAIMS_DIR; + else process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir; + fs.rmSync(claimsRoot, { recursive: true, force: true }); + } +}); + +test('a successful close clears the device claim', async () => { + const claimsRoot = mkdtempForTestSync('agent-device-session-close-claim-cleared-'); + const previousClaimsDir = process.env.AGENT_DEVICE_CLAIMS_DIR; + process.env.AGENT_DEVICE_CLAIMS_DIR = claimsRoot; + try { + const sessionStore = makeSessionStore(); + const sessionName = 'targeted-close-claim-cleared-session'; + const device = { + platform: 'android' as const, + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator' as const, + booted: true, + }; + const acquired = await acquireAdvisoryDeviceClaim({ + device, + session: sessionName, + workspace: process.cwd(), + stateDir: sessionStore.resolveDaemonStateDir(), + }); + if (!acquired.ownership) { + throw new Error('expected the test session to acquire a device claim'); + } + const session = { + ...makeSession(sessionName, device), + deviceClaim: acquired.ownership, + }; + sessionStore.set(sessionName, session); + + const response = await handleSessionCommands({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: [], + flags: {}, + }, + sessionName, + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + + expect(response?.ok).toBe(true); + expect(sessionStore.get(sessionName)).toBeUndefined(); + expect(inspectDeviceClaims({ serial: device.id })).toEqual([]); + } finally { + if (previousClaimsDir === undefined) delete process.env.AGENT_DEVICE_CLAIMS_DIR; + else process.env.AGENT_DEVICE_CLAIMS_DIR = previousClaimsDir; + fs.rmSync(claimsRoot, { recursive: true, force: true }); + } +}); + test('targeted close skips platform dispatch and preserves the error when the required pre-close runner stop fails', async () => { const sessionStore = makeSessionStore(); const sessionName = 'targeted-close-preclose-failure-session'; diff --git a/src/daemon/handlers/interaction-ios-tap-outcome.ts b/src/daemon/handlers/interaction-ios-tap-outcome.ts index 526b755c9..9334acd5a 100644 --- a/src/daemon/handlers/interaction-ios-tap-outcome.ts +++ b/src/daemon/handlers/interaction-ios-tap-outcome.ts @@ -223,9 +223,9 @@ function matchingCaptureFlags( flags: CommandFlags | undefined, presentation: SnapshotPresentation | undefined, ): CommandFlags | undefined { - if (!flags) return undefined; + if (!flags && !presentation) return undefined; return { - ...flags, + ...(flags ?? {}), out: undefined, ...(presentation ? { diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index f652fb844..8fe47a71c 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -406,7 +406,18 @@ async function runCloseTeardownAndRelease(params: { failures: cleanupFailures, }); const deviceClaimBlockingError = platformCloseError ?? cleanupAggregate; - if (!deviceClaimBlockingError) { + if (deviceClaimBlockingError) { + if (session.deviceClaim) { + emitDiagnostic({ + level: 'warn', + phase: 'device_claim_close_effects_unconfirmed', + data: { + deviceKey: session.deviceClaim.deviceKey, + session: sessionName, + }, + }); + } + } else { await clearAdvisoryDeviceClaim(session.deviceClaim); } sessionStore.delete(sessionName); diff --git a/src/platforms/android/__tests__/device-input-state.test.ts b/src/platforms/android/__tests__/device-input-state.test.ts index 01493fe4e..484cba636 100644 --- a/src/platforms/android/__tests__/device-input-state.test.ts +++ b/src/platforms/android/__tests__/device-input-state.test.ts @@ -7,6 +7,7 @@ import { dismissAndroidKeyboard, getAndroidKeyboardState, getAndroidKeyboardStatusWithAdb, + writeAndroidClipboardWithAdb, } from '../device-input-state.ts'; import { flushDiagnosticsToSessionFile, withDiagnosticsScope } from '../../../utils/diagnostics.ts'; import { withScriptedAdb } from '../../../__tests__/test-utils/mocked-binaries.ts'; @@ -269,6 +270,30 @@ test('getAndroidKeyboardState treats stale input view as hidden when the IME win ); }); +test('writeAndroidClipboardWithAdb shell-quotes text containing metacharacters', async () => { + const calls: string[][] = []; + const adb: AndroidAdbExecutor = async (args) => { + calls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }; + + await writeAndroidClipboardWithAdb(adb, 'otp; echo pwned'); + + assert.deepEqual(calls, [['shell', 'cmd', 'clipboard', 'set', 'text', "'otp; echo pwned'"]]); +}); + +test('writeAndroidClipboardWithAdb leaves safe text unquoted', async () => { + const calls: string[][] = []; + const adb: AndroidAdbExecutor = async (args) => { + calls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }; + + await writeAndroidClipboardWithAdb(adb, 'android-otp'); + + assert.deepEqual(calls, [['shell', 'cmd', 'clipboard', 'set', 'text', 'android-otp']]); +}); + test('dismissAndroidKeyboard skips keyevent when keyboard is already hidden', async () => { await withScriptedAdb( 'agent-device-android-keyboard-dismiss-hidden-', diff --git a/src/platforms/android/__tests__/input-actions.test.ts b/src/platforms/android/__tests__/input-actions.test.ts index 083d8fd26..14f7e75fc 100644 --- a/src/platforms/android/__tests__/input-actions.test.ts +++ b/src/platforms/android/__tests__/input-actions.test.ts @@ -208,6 +208,47 @@ test('typeAndroid sends one character at a time when delay is requested', async ); }); +test('typeAndroid shell-quotes text containing shell metacharacters', async () => { + await withScriptedAdb( + 'agent-device-android-type-shell-metacharacters-', + [ + '#!/bin/sh', + 'printf "__CMD__\\n" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'printf "%s\\n" "$@" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'exit 0', + '', + ].join('\n'), + async ({ argsLogPath, device }) => { + await typeAndroid(device, 'otp; echo pwned'); + const logged = await fs.readFile(argsLogPath, 'utf8'); + // The chunk carrying `;` is single-quoted so the device shell cannot + // re-tokenize it into a second command. + assert.match(logged, /shell\ninput\ntext\n'otp;%sech'/); + // The next chunk has no shell-significant characters and stays unquoted. + assert.match(logged, /shell\ninput\ntext\no%spwned\n/); + }, + ); +}); + +test('typeAndroid leaves safe text unquoted', async () => { + await withScriptedAdb( + 'agent-device-android-type-safe-unquoted-', + [ + '#!/bin/sh', + 'printf "__CMD__\\n" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'printf "%s\\n" "$@" >> "$AGENT_DEVICE_TEST_ARGS_FILE"', + 'exit 0', + '', + ].join('\n'), + async ({ argsLogPath, device }) => { + await typeAndroid(device, 'hello'); + const logged = await fs.readFile(argsLogPath, 'utf8'); + assert.match(logged, /shell\ninput\ntext\nhello\n/); + assert.doesNotMatch(logged, /shell\ninput\ntext\n'/); + }, + ); +}); + test('fillAndroid uses chunk-safe shell input and retries when verification still fails', async () => { await withScriptedAdb( 'agent-device-android-fill-fallback-', diff --git a/src/platforms/android/app-lifecycle.ts b/src/platforms/android/app-lifecycle.ts index 1dc4b17ef..97a78a842 100644 --- a/src/platforms/android/app-lifecycle.ts +++ b/src/platforms/android/app-lifecycle.ts @@ -7,6 +7,7 @@ import { sleep } from '../../utils/timeouts.ts'; import type { AppsFilter } from '@agent-device/contracts/device'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { isDeepLinkTarget } from '@agent-device/contracts/command'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { createAppResolutionCache, type AppResolutionCacheScope } from '../app-resolution-cache.ts'; import { waitForAndroidBoot } from './devices.ts'; import { runAndroidAdb } from './adb.ts'; @@ -306,13 +307,8 @@ export type OpenAndroidAppOptions = { // characters, so they round-trip untouched. URLs and launch arguments are // user-supplied and may contain JSON, spaces, `#`, or `&`; each is single-quoted // unless it consists entirely of safe shell characters. -function quoteAndroidShellArg(arg: string): string { - if (/^[A-Za-z0-9_@%+=:,./-]+$/.test(arg)) return arg; - return `'${arg.replace(/'/g, `'\\''`)}'`; -} - function androidLaunchArgs(options: OpenAndroidAppOptions): string[] { - return (options.launchArgs ?? []).map(quoteAndroidShellArg); + return (options.launchArgs ?? []).map(shellQuoteIfNeeded); } export async function openAndroidApp( @@ -367,7 +363,7 @@ async function openAndroidDeepLink( '-a', 'android.intent.action.VIEW', '-d', - quoteAndroidShellArg(target), + shellQuoteIfNeeded(target), ...androidDeepLinkPackageArgs(options.appBundleId), ...androidLaunchArgs(options), ]); @@ -398,7 +394,7 @@ async function openAndroidAppBoundDeepLink( '-a', 'android.intent.action.VIEW', '-d', - quoteAndroidShellArg(deepLinkUrl), + shellQuoteIfNeeded(deepLinkUrl), '-p', resolved, ...androidLaunchArgs(options), diff --git a/src/platforms/android/device-input-state.ts b/src/platforms/android/device-input-state.ts index 61d25630c..cec37ffd6 100644 --- a/src/platforms/android/device-input-state.ts +++ b/src/platforms/android/device-input-state.ts @@ -1,6 +1,7 @@ import { emitDiagnostic } from '../../utils/diagnostics.ts'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { isClipboardShellUnsupported, sleep } from './adb.ts'; import { androidAdbResultError, @@ -308,7 +309,7 @@ export async function writeAndroidClipboardWithAdb( ): Promise { await runAndroidClipboardShellCommand( adb, - ['shell', 'cmd', 'clipboard', 'set', 'text', text], + ['shell', 'cmd', 'clipboard', 'set', 'text', shellQuoteIfNeeded(text)], 'write', ); } diff --git a/src/platforms/android/input-actions.ts b/src/platforms/android/input-actions.ts index 067ab48a2..1478ad78b 100644 --- a/src/platforms/android/input-actions.ts +++ b/src/platforms/android/input-actions.ts @@ -10,6 +10,7 @@ import { import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import { shellQuoteIfNeeded } from '../../utils/shell-quote.ts'; import { resolveAndroidAdbExecutor, resolveAndroidTextInjector, @@ -398,7 +399,12 @@ async function typeAndroidShell( async function typeAndroidShellChunk(device: DeviceInfo, text: string): Promise { if (!text) return; try { - await runAndroidAdb(device, ['shell', 'input', 'text', encodeAndroidInputText(text)]); + await runAndroidAdb(device, [ + 'shell', + 'input', + 'text', + shellQuoteIfNeeded(encodeAndroidInputText(text)), + ]); } catch (error) { if (isAndroidInputTextUnsupported(error)) { throw unsupportedAndroidShellTextError(text, error); diff --git a/src/remote/__tests__/remote-connection-state.test.ts b/src/remote/__tests__/remote-connection-state.test.ts new file mode 100644 index 000000000..96587bca7 --- /dev/null +++ b/src/remote/__tests__/remote-connection-state.test.ts @@ -0,0 +1,102 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { mkdtempForTest } from '../../__tests__/test-utils/tmp-dir.ts'; +import { + buildRemoteConnectionDaemonState, + hashRemoteConfigFile, + resolveRemoteConnectionDefaults, + writeRemoteConnectionState, + type RemoteConnectionState, +} from '../remote-connection-state.ts'; + +// Regression coverage for ADR 0007: generated connection profiles must strip +// the daemon bearer token. `connect` used to write it straight into the +// persisted connection-state file; these tests guard against that shadow +// coming back. + +const FAKE_DAEMON_TOKEN = 'test-not-a-real-daemon-token'; + +test('buildRemoteConnectionDaemonState does not persist the daemon auth token', () => { + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + + assert.equal(Object.hasOwn(daemon ?? {}, 'authToken'), false); + assert.equal(daemon?.baseUrl, 'https://daemon.example.test'); + assert.equal(daemon?.transport, 'http'); + assert.equal(daemon?.serverMode, 'http'); +}); + +test('written connection state contains no daemon auth token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-write-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: FAKE_DAEMON_TOKEN, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-write-test', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + + writeRemoteConnectionState({ stateDir, state }); + + const writtenPath = path.join(stateDir, 'remote-connections', 'adc-write-test.json'); + const written = fs.readFileSync(writtenPath, 'utf8'); + assert.equal(written.includes(FAKE_DAEMON_TOKEN), false); + assert.equal(written.includes('authToken'), false); +}); + +test('resolveRemoteConnectionDefaults falls back to the environment token', async () => { + const tempRoot = await mkdtempForTest('agent-device-remote-connection-state-defaults-'); + const stateDir = path.join(tempRoot, '.state'); + const remoteConfigPath = path.join(tempRoot, 'remote.json'); + fs.writeFileSync(remoteConfigPath, '{}'); + + const daemon = buildRemoteConnectionDaemonState({ + daemonBaseUrl: 'https://daemon.example.test', + daemonAuthToken: undefined, + daemonTransport: 'http', + daemonServerMode: 'http', + }); + const now = new Date().toISOString(); + const state: RemoteConnectionState = { + version: 1, + session: 'adc-env-fallback', + remoteConfigPath, + remoteConfigHash: hashRemoteConfigFile(remoteConfigPath), + daemon, + tenant: 'acme', + runId: 'run-1', + connectedAt: now, + updatedAt: now, + }; + writeRemoteConnectionState({ stateDir, state }); + + const defaults = resolveRemoteConnectionDefaults({ + stateDir, + session: 'adc-env-fallback', + cwd: tempRoot, + env: { AGENT_DEVICE_DAEMON_AUTH_TOKEN: FAKE_DAEMON_TOKEN }, + }); + + assert.equal(defaults?.flags.daemonAuthToken, FAKE_DAEMON_TOKEN); +}); diff --git a/src/remote/remote-connection-state.ts b/src/remote/remote-connection-state.ts index 81bae0e3f..89f5dea1b 100644 --- a/src/remote/remote-connection-state.ts +++ b/src/remote/remote-connection-state.ts @@ -19,7 +19,6 @@ export type RemoteConnectionState = { remoteConfigHash: string; daemon?: { baseUrl?: string; - authToken?: string; transport?: CliFlags['daemonTransport']; serverMode?: CliFlags['daemonServerMode']; }; @@ -94,7 +93,6 @@ export function buildRemoteConnectionDaemonState( ): RemoteConnectionState['daemon'] { return { baseUrl: sanitizeDaemonBaseUrl(flags.daemonBaseUrl), - authToken: flags.daemonAuthToken, transport: flags.daemonTransport, serverMode: flags.daemonServerMode, }; @@ -154,7 +152,11 @@ export function resolveRemoteConnectionDefaults(options: { ...profile, remoteConfig: state.remoteConfigPath, daemonBaseUrl: state.daemon?.baseUrl ?? profile.daemonBaseUrl, - daemonAuthToken: state.daemon?.authToken ?? profile.daemonAuthToken, + // Deliberately not sourced from state: the daemon bearer token is never + // persisted to the connection-state file (ADR 0007). It is resolved + // from the profile here, and from the flag/env/CLI-session chain in + // resolveRemoteAuth (src/cli/auth-session.ts) at command dispatch time. + daemonAuthToken: profile.daemonAuthToken, daemonTransport: state.daemon?.transport ?? profile.daemonTransport, daemonServerMode: state.daemon?.serverMode ?? profile.daemonServerMode, ...leaseScopeToCommandFlags(leaseScope), diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 6af601164..b7257a2b2 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -1482,7 +1482,7 @@ function assertAndroidPushAndEventContract(world: AndroidSettingsWorld): void { 'com.example.demo', ]); assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'get', 'text']); - assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'set', 'text', 'android otp']); + assertCommandCall(adbCalls, ['shell', 'cmd', 'clipboard', 'set', 'text', "'android otp'"]); assertCommandCall(adbCalls, ['shell', 'dumpsys', 'input_method']); } diff --git a/test/integration/provider-scenarios/android-world.ts b/test/integration/provider-scenarios/android-world.ts index 0113dc254..e1a87b68d 100644 --- a/test/integration/provider-scenarios/android-world.ts +++ b/test/integration/provider-scenarios/android-world.ts @@ -263,16 +263,33 @@ function createAndroidProviderShellState(): AndroidProviderShellState { return { searchText: '', clipboardText: 'hello' }; } +const ANDROID_CLIPBOARD_SET_TEXT_PREFIX = ['shell', 'cmd', 'clipboard', 'set', 'text']; + function updateAndroidProviderShellState(args: string[], state: AndroidProviderShellState): void { if (args[0] === 'shell' && args[1] === 'input' && args[2] === 'text') { state.searchText = String(args[3] ?? '').replaceAll('%s', ' '); return; } - if (args.join(' ') === 'shell cmd clipboard set text android otp') { - state.clipboardText = 'android otp'; + if (argsStartWith(args, ANDROID_CLIPBOARD_SET_TEXT_PREFIX)) { + state.clipboardText = unquoteAndroidShellArg( + String(args[ANDROID_CLIPBOARD_SET_TEXT_PREFIX.length] ?? ''), + ); } } +function argsStartWith(args: string[], prefix: string[]): boolean { + return prefix.every((value, index) => args[index] === value); +} + +// The real device shell unwraps a single-quoted argument (and collapses the +// `'\''` escape back to `'`) before `cmd` ever sees it, so this harness has +// to mirror that unwrap to keep modelling what the device actually receives +// — the inverse of the quoting in src/utils/shell-quote.ts. +function unquoteAndroidShellArg(value: string): string { + if (!value.startsWith("'") || !value.endsWith("'") || value.length < 2) return value; + return value.slice(1, -1).replaceAll("'\\''", "'"); +} + function androidDeviceStateAdbResult( key: string, args: string[], diff --git a/test/integration/smoke-provider-cli-disconnect.test.ts b/test/integration/smoke-provider-cli-disconnect.test.ts index ff739cd15..de8e3daec 100644 --- a/test/integration/smoke-provider-cli-disconnect.test.ts +++ b/test/integration/smoke-provider-cli-disconnect.test.ts @@ -48,6 +48,10 @@ function createProviderEnv(fixture: ProviderDaemonFixture): NodeJS.ProcessEnv { BROWSERSTACK_USERNAME: 'browser-user', BROWSERSTACK_ACCESS_KEY: 'browser-key', AGENT_DEVICE_TEST_RPC_LOG_PATH: fixture.rpcLogPath, + // Generated connection state never persists the daemon bearer token + // (ADR 0007), so commands after `connect` must resolve it from the + // environment/CLI chain, same as real usage. + AGENT_DEVICE_DAEMON_AUTH_TOKEN: 'test-daemon-token', NODE_OPTIONS: [process.env.NODE_OPTIONS, `--import=${fetchFixtureUrl}`] .filter(Boolean) .join(' '), diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 869acfa15..bf97512b6 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -31,12 +31,11 @@ By default the proxy binds `127.0.0.1`. Use `--host 0.0.0.0` only when you inten ## Remote Client -On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token: +On the machine running the agent, connect to the public tunnel origin with the `/agent-device` base path and the printed token. The generated connection profile never stores the token (only routing metadata), so export it once and every command in the session picks it up: ```bash -agent-device connect proxy \ - --daemon-base-url https://example.trycloudflare.com/agent-device \ - --daemon-auth-token +export AGENT_DEVICE_DAEMON_AUTH_TOKEN= +agent-device connect proxy --daemon-base-url https://example.trycloudflare.com/agent-device agent-device devices --platform ios agent-device open MyApp --platform ios agent-device snapshot --platform ios @@ -44,6 +43,8 @@ agent-device close agent-device disconnect ``` +Passing `--daemon-auth-token ` instead of exporting the environment variable also works, but only authenticates the single command it is passed to; subsequent commands need the token again through the env var, a `daemonAuthToken` entry in your remote config profile, or a repeated `--daemon-auth-token` flag. + `connect proxy` stores the proxy profile and client identity. Device leases are automatic on `open` and expire after five minutes without commands. `close` releases the active session and device lease; `disconnect` clears local connection state. Multiple agents can share one proxy when each uses the normal `connect proxy`, `open`, commands, `close`, and `disconnect` flow. A busy device error means another agent owns the device until it closes or its inactivity lease expires.