From c9065938ed359250f7ee35947b4750e43903c2b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 19:27:53 +0200 Subject: [PATCH 1/8] fix(android): warn when a permission revoke kills the session app settings permission deny|reset maps to pm revoke, and Android kills the app's process whenever a runtime permission it currently holds is revoked, so a grant -> deny/reset sequence silently left the session on the launcher and the next selector failed with no hint. The revoke path now reads the prior grant state from dumpsys package first and, when it was granted, returns wasGranted: true plus a warning naming open --relaunch; the settings CLI output renders response warnings, and commands.md documents the behavior next to the pm revoke mapping. Closes #1796 --- src/commands/capture/index.test.ts | 24 +++- src/commands/capture/settings.ts | 5 +- src/commands/output-common.ts | 17 +++ .../android/__tests__/settings.test.ts | 106 ++++++++++++++- src/platforms/android/settings.ts | 125 +++++++++++++++--- website/docs/docs/commands.md | 1 + 6 files changed, 253 insertions(+), 25 deletions(-) diff --git a/src/commands/capture/index.test.ts b/src/commands/capture/index.test.ts index d7f9af193..de31ab2eb 100644 --- a/src/commands/capture/index.test.ts +++ b/src/commands/capture/index.test.ts @@ -4,7 +4,7 @@ import { alertCliReader, alertDaemonWriter } from './alert.ts'; import { diffCliReader } from './diff.ts'; import { snapshotCliOutput } from './output.ts'; import { screenshotCliReader, screenshotDaemonWriter } from './screenshot.ts'; -import { settingsCliReader, settingsDaemonWriter } from './settings.ts'; +import { settingsCliReader, settingsCommandFacet, settingsDaemonWriter } from './settings.ts'; import { snapshotCliReader } from './snapshot.ts'; import { waitCliReader, waitDaemonWriter } from './wait.ts'; @@ -187,4 +187,26 @@ describe('capture command interface', () => { positionals: ['permission', 'grant', 'camera', 'limited'], }); }); + + // #1796: the Android revoke warning rides `warnings`; the human CLI line must show it. + test('settings CLI output renders response warnings after the message', () => { + const warning = 'Revoking android.permission.CAMERA while it was granted made Android kill …'; + const output = settingsCommandFacet.cliOutputFormatter!({ + input: {}, + result: { setting: 'permission', state: 'reset', message: 'Updated setting: permission' }, + }); + expect(output.text).toBe('Updated setting: permission'); + + const warned = settingsCommandFacet.cliOutputFormatter!({ + input: {}, + result: { + setting: 'permission', + state: 'reset', + message: 'Updated setting: permission', + warnings: [warning], + }, + }); + expect(warned.text).toBe(`Updated setting: permission\nWarning: ${warning}`); + expect(warned.data).toMatchObject({ warnings: [warning] }); + }); }); diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index 0c47ddfeb..d7c0436fe 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -17,6 +17,7 @@ import { import type { CliReader, DaemonWriter } from '../cli-grammar/types.ts'; import { defineCommandFacet } from '../family/types.ts'; import { defineFieldCommandMetadata } from '../field-command-contract.ts'; +import { messageWithWarningsOutput } from '../output-common.ts'; const SETTINGS_COMMAND_NAME = 'settings'; const settingsCommandDescription = @@ -59,13 +60,15 @@ export const settingsCommandFacet = defineCommandFacet({ text: { summary: 'Change OS settings and app permissions', cliDetail: - 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app.', + 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills the app process; the response warns, and open --relaunch restores it.', }, metadata: settingsCommandMetadata, definition: settingsCommandDefinition, cliSchema: settingsCliSchema, cliReader: settingsCliReader, daemonWriter: settingsDaemonWriter, + // Android permission revokes append a relaunch warning (#1796); render it for humans too. + cliOutputFormatter: messageWithWarningsOutput, }); // fallow-ignore-next-line complexity diff --git a/src/commands/output-common.ts b/src/commands/output-common.ts index 8597302ba..53125ae60 100644 --- a/src/commands/output-common.ts +++ b/src/commands/output-common.ts @@ -18,6 +18,23 @@ export function messageCliOutput(result: Record): CliOutput { return { data: result, text: readCommandMessage(result) }; } +/** + * `messageCliOutput` plus one `Warning:` line per entry of the response's `warnings` + * array — the composable warnings channel (`open`, `debug`, snapshot capture use it too), + * so a warning the daemon appended reaches the human CLI reader, not only `--json`. + */ +export const messageWithWarningsOutput = resultOutput( + (result: Record): CliOutput => { + const output = messageCliOutput(result); + const warnings = Array.isArray(result.warnings) + ? result.warnings.filter((warning): warning is string => typeof warning === 'string') + : []; + if (warnings.length === 0) return output; + const lines = [output.text, ...warnings.map((warning) => `Warning: ${warning}`)]; + return { data: output.data, text: lines.filter(Boolean).join('\n') }; + }, +); + /** * ADR 0014: a reusable ref in a PARTIAL result renders in ready-to-copy * `@eN~s` form so a human CLI caller can paste it into the next diff --git a/src/platforms/android/__tests__/settings.test.ts b/src/platforms/android/__tests__/settings.test.ts index 336579af2..3f02fec1b 100644 --- a/src/platforms/android/__tests__/settings.test.ts +++ b/src/platforms/android/__tests__/settings.test.ts @@ -1,6 +1,10 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { setAndroidSetting } from '../settings.ts'; +import { + androidRevokedGrantedPermissionWarning, + parseAndroidGrantedRuntimePermissions, + setAndroidSetting, +} from '../settings.ts'; import { ANDROID_EMULATOR, assertRejectsAppError, @@ -177,6 +181,106 @@ test('setAndroidSetting permission reset notifications clears permission flags f ); }); +// #1796: Android kills the app when a permission it holds is revoked. The prior grant state is +// read before `pm revoke`, and a granted -> revoked transition surfaces as a warning + typed field. +const DUMPSYS_MICROPHONE_GRANTED = [ + 'Packages:', + ' Package [com.example.app] (abc):', + ' User 0: ceDataInode=0 installed=true', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET|USER_SENSITIVE_WHEN_GRANTED]', + ' android.permission.CAMERA: granted=false, flags=[ USER_SENSITIVE_WHEN_GRANTED]', +].join('\n'); + +test.each(['deny', 'reset'] as const)( + 'setAndroidSetting permission %s warns that revoking a granted permission killed the app', + async (action) => { + await withFakeAdb( + (args) => + args.join(' ') === 'shell dumpsys package com.example.app' + ? DUMPSYS_MICROPHONE_GRANTED + : undefined, + async ({ calls, device }) => { + const result = await setAndroidSetting(device, 'permission', action, 'com.example.app', { + permissionTarget: 'microphone', + }); + const flat = calls.map((args) => args.join(' ')); + // The state is read BEFORE the revoke: after it, dumpsys would already say false. + assert.ok( + flat.indexOf('shell dumpsys package com.example.app') < + flat.indexOf('shell pm revoke com.example.app android.permission.RECORD_AUDIO'), + flat.join('; '), + ); + assert.deepEqual(result, { + permission: 'android.permission.RECORD_AUDIO', + wasGranted: true, + warnings: [ + androidRevokedGrantedPermissionWarning( + 'com.example.app', + 'android.permission.RECORD_AUDIO', + ), + ], + }); + assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); + }, + ); + }, +); + +test('setAndroidSetting permission deny stays quiet when the permission was not granted', async () => { + await withFakeAdb( + (args) => + args.join(' ') === 'shell dumpsys package com.example.app' + ? DUMPSYS_MICROPHONE_GRANTED + : undefined, + async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'camera', + }); + assert.deepEqual(result, { permission: 'android.permission.CAMERA', wasGranted: false }); + }, + ); +}); + +test('setAndroidSetting permission grant does not read grant state', async () => { + await withFakeAdb( + () => undefined, + async ({ calls, device }) => { + const result = await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'camera', + }); + assert.equal(result, undefined); + assert.deepEqual(calls, [ + ['shell', 'pm', 'grant', 'com.example.app', 'android.permission.CAMERA'], + ]); + }, + ); +}); + +test('setAndroidSetting permission reset notifications warns when POST_NOTIFICATIONS was granted', async () => { + await withFakeAdb( + (args) => + args.join(' ') === 'shell dumpsys package com.example.app' + ? ' runtime permissions:\n android.permission.POST_NOTIFICATIONS: granted=true, flags=[ USER_SET]' + : undefined, + async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { + permissionTarget: 'notifications', + }); + assert.equal(result?.wasGranted, true); + assert.equal(Array.isArray(result?.warnings), true); + }, + ); +}); + +test('parseAndroidGrantedRuntimePermissions reads only granted=true runtime permissions', () => { + assert.deepEqual( + [...parseAndroidGrantedRuntimePermissions(DUMPSYS_MICROPHONE_GRANTED)], + ['android.permission.RECORD_AUDIO'], + ); + assert.deepEqual([...parseAndroidGrantedRuntimePermissions('')], []); +}); + test('setAndroidSetting permission reset camera clears permission flags for reprompt', async () => { await withFakeAdb( () => undefined, diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index a7dd34067..8d0e40c89 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -142,34 +142,115 @@ export async function setAndroidSetting( if (!appPackage) { throw new AppError('INVALID_ARGS', 'permission setting requires an active app in session'); } - const action = parsePermissionAction(state); - const target = parseAndroidPermissionTarget( - options?.permissionTarget, - options?.permissionMode, - ); - if (target.kind === 'notifications') { - await setAndroidNotificationPermission(device, appPackage, action, target); - return; - } - const pmAction = action === 'grant' ? 'grant' : 'revoke'; - if (target.type === 'photos') { - const permission = await setAndroidPhotoPermission(device, appPackage, pmAction); - if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, permission); - } - return; - } - await runAndroidAdb(device, ['shell', 'pm', pmAction, appPackage, target.value]); - if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, target.value); - } - return; + return await setAndroidPermission(device, appPackage, state, options); } default: throw new AppError('INVALID_ARGS', `Unsupported setting: ${setting}`); } } +/** + * Android kills the app's process whenever a runtime permission it currently holds is + * revoked (`pm revoke` after a grant, foreground or background), so a `deny`/`reset` that + * follows a grant leaves the session pointing at a dead app and the next selector fails + * against the launcher (#1796). Revoking a permission the app does not hold is harmless. + * The prior grant state is read before the revoke so the response can say so. + */ +export function androidRevokedGrantedPermissionWarning( + appPackage: string, + permission: string, +): string { + return ( + `Revoking ${permission} while it was granted made Android kill ${appPackage}; the app is ` + + `no longer running. Relaunch it with open ${appPackage} --relaunch before the next interaction.` + ); +} + +type AndroidPermissionTarget = ReturnType; + +async function setAndroidPermission( + device: DeviceInfo, + appPackage: string, + state: string, + options: SettingOptions | undefined, +): Promise | void> { + const action = parsePermissionAction(state); + const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); + if (action === 'grant') { + await grantAndroidPermission(device, appPackage, target); + return; + } + const grantedBefore = await readAndroidGrantedRuntimePermissions(device, appPackage); + const permission = await revokeAndroidPermission(device, appPackage, action, target); + const wasGranted = grantedBefore.has(permission); + return { + permission, + wasGranted, + ...(wasGranted + ? { warnings: [androidRevokedGrantedPermissionWarning(appPackage, permission)] } + : {}), + }; +} + +async function grantAndroidPermission( + device: DeviceInfo, + appPackage: string, + target: AndroidPermissionTarget, +): Promise { + if (target.kind === 'notifications') { + await setAndroidNotificationPermission(device, appPackage, 'grant', target); + } else if (target.type === 'photos') { + await setAndroidPhotoPermission(device, appPackage, 'grant'); + } else { + await runAndroidAdb(device, ['shell', 'pm', 'grant', appPackage, target.value]); + } +} + +/** Revokes (and for `reset`, clears the flags of) the target; returns the permission revoked. */ +async function revokeAndroidPermission( + device: DeviceInfo, + appPackage: string, + action: 'deny' | 'reset', + target: AndroidPermissionTarget, +): Promise { + if (target.kind === 'notifications') { + await setAndroidNotificationPermission(device, appPackage, action, target); + return target.permission; + } + let permission: string; + if (target.type === 'photos') { + permission = await setAndroidPhotoPermission(device, appPackage, 'revoke'); + } else { + permission = target.value; + await runAndroidAdb(device, ['shell', 'pm', 'revoke', appPackage, permission]); + } + if (action === 'reset') await clearAndroidPermissionFlags(device, appPackage, permission); + return permission; +} + +/** + * Runtime permissions `dumpsys package ` reports as `granted=true`. Best-effort: a + * failed or unparseable dump reads as "nothing granted", which only costs the warning. + */ +async function readAndroidGrantedRuntimePermissions( + device: DeviceInfo, + appPackage: string, +): Promise> { + const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { + allowFailure: true, + }); + if (result.exitCode !== 0) return new Set(); + return parseAndroidGrantedRuntimePermissions(result.stdout); +} + +export function parseAndroidGrantedRuntimePermissions(dumpsysOutput: string): ReadonlySet { + const granted = new Set(); + for (const match of dumpsysOutput.matchAll(/^\s*([\w.]+): granted=true\b/gm)) { + granted.add(match[1]!); + } + return granted; +} + type AndroidFingerprintAction = 'match' | 'nonmatch'; function parseAndroidFingerprintAction(state: string): AndroidFingerprintAction { diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index b4dab1963..80ddf3971 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -674,6 +674,7 @@ agent-device settings permission reset screen-recording --platform macos - On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions. - On macOS, `settings permission deny ...` is intentionally unsupported. - Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. +- Android kills the app process whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app not running. The response then carries `wasGranted: true` and a warning; relaunch with `open --relaunch` before the next interaction. Revoking a permission the app does not hold is harmless. - `full|limited` mode is supported only for iOS `photos`; other targets reject mode. - Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes. From d9a62553f682ce4ce0e4d2c2ae03203a5995e194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 08:05:46 +0200 Subject: [PATCH 2/8] fix(android): state the revoke-kill consequence conditionally Review finding: the warning asserted the app had been killed, inferred only from the prior grant state. dumpsys reports granted=true for any user profile while pm revoke acts on the current one, and the app need not have been running, so the claim could be false. State the platform rule and make the consequence conditional; the relaunch guidance is unchanged. --- src/commands/capture/index.test.ts | 2 +- src/commands/capture/settings.ts | 2 +- src/platforms/android/__tests__/settings.test.ts | 4 ++++ src/platforms/android/settings.ts | 11 ++++++++--- website/docs/docs/commands.md | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/commands/capture/index.test.ts b/src/commands/capture/index.test.ts index de31ab2eb..b188c2d80 100644 --- a/src/commands/capture/index.test.ts +++ b/src/commands/capture/index.test.ts @@ -190,7 +190,7 @@ describe('capture command interface', () => { // #1796: the Android revoke warning rides `warnings`; the human CLI line must show it. test('settings CLI output renders response warnings after the message', () => { - const warning = 'Revoking android.permission.CAMERA while it was granted made Android kill …'; + const warning = 'android.permission.CAMERA was granted before this revoke, and Android …'; const output = settingsCommandFacet.cliOutputFormatter!({ input: {}, result: { setting: 'permission', state: 'reset', message: 'Updated setting: permission' }, diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index d7c0436fe..f5b70ab4a 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -60,7 +60,7 @@ export const settingsCommandFacet = defineCommandFacet({ text: { summary: 'Change OS settings and app permissions', cliDetail: - 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills the app process; the response warns, and open --relaunch restores it.', + 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills a running app; the response warns, and open --relaunch restores it.', }, metadata: settingsCommandMetadata, definition: settingsCommandDefinition, diff --git a/src/platforms/android/__tests__/settings.test.ts b/src/platforms/android/__tests__/settings.test.ts index 3f02fec1b..c08a514f2 100644 --- a/src/platforms/android/__tests__/settings.test.ts +++ b/src/platforms/android/__tests__/settings.test.ts @@ -222,6 +222,10 @@ test.each(['deny', 'reset'] as const)( ], }); assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); + // The warning states the platform rule and keeps the consequence conditional: the + // prior-grant read proves neither that the app was running nor that the grant was + // the current user's (#1796 review). + assert.match(String(result?.warnings), /if com\.example\.app was running it is no longer/); }, ); }, diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index 8d0e40c89..67b61afdd 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -154,15 +154,20 @@ export async function setAndroidSetting( * revoked (`pm revoke` after a grant, foreground or background), so a `deny`/`reset` that * follows a grant leaves the session pointing at a dead app and the next selector fails * against the launcher (#1796). Revoking a permission the app does not hold is harmless. - * The prior grant state is read before the revoke so the response can say so. + * + * The prior grant state is read before the revoke; process death itself is NOT observed + * (that would be option (b) in the issue), and the read cannot prove the app was running + * or that the grant belonged to the current user profile. The wording therefore states the + * platform rule and makes the consequence conditional rather than asserting a death. */ export function androidRevokedGrantedPermissionWarning( appPackage: string, permission: string, ): string { return ( - `Revoking ${permission} while it was granted made Android kill ${appPackage}; the app is ` + - `no longer running. Relaunch it with open ${appPackage} --relaunch before the next interaction.` + `${permission} was granted before this revoke, and Android kills an app when a granted ` + + `permission is revoked: if ${appPackage} was running it is no longer. Relaunch it with ` + + `open ${appPackage} --relaunch before the next interaction.` ); } diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 80ddf3971..f8c31c8b1 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -674,7 +674,7 @@ agent-device settings permission reset screen-recording --platform macos - On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions. - On macOS, `settings permission deny ...` is intentionally unsupported. - Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. -- Android kills the app process whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app not running. The response then carries `wasGranted: true` and a warning; relaunch with `open --relaunch` before the next interaction. Revoking a permission the app does not hold is harmless. +- Android kills a running app whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app no longer running. The response then carries `wasGranted: true` and a warning; relaunch with `open --relaunch` before the next interaction. Revoking a permission the app does not hold is harmless. - `full|limited` mode is supported only for iOS `photos`; other targets reject mode. - Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes. From 960c455f04ddfcc0e5dd5502eeeb3bb6200a0a95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 13:29:52 +0200 Subject: [PATCH 3/8] fix(android): model the prior grant state as granted/not_granted/unknown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed or unparseable dumpsys read as "nothing granted", so the response asserted the app was untouched when the state was simply unknown, and the grant scan matched every granted=true line — install permissions and other users' blocks included — so another profile's grant could claim a kill that never happened. Both directions of the same defect. The read now resolves the acting user (am get-current-user) and walks the dump's nesting (Packages: > User : > runtime permissions:), and reports priorGrantState: granted | not_granted | unknown. unknown carries the same relaunch guidance without claiming what the state was; only not_granted is silent. --- src/commands/capture/settings.ts | 2 +- .../__tests__/permission-grant-state.test.ts | 132 +++++++++++++ .../android/__tests__/settings.test.ts | 187 +++++++++++++----- .../android/permission-grant-state.ts | 128 ++++++++++++ src/platforms/android/settings.ts | 65 +++--- website/docs/docs/commands.md | 2 +- 6 files changed, 424 insertions(+), 92 deletions(-) create mode 100644 src/platforms/android/__tests__/permission-grant-state.test.ts create mode 100644 src/platforms/android/permission-grant-state.ts diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index f5b70ab4a..98f1255fa 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -60,7 +60,7 @@ export const settingsCommandFacet = defineCommandFacet({ text: { summary: 'Change OS settings and app permissions', cliDetail: - 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills a running app; the response warns, and open --relaunch restores it.', + 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills a running app; the response reports priorGrantState (granted|not_granted|unknown) and warns for granted and unknown, with open --relaunch to restore it.', }, metadata: settingsCommandMetadata, definition: settingsCommandDefinition, diff --git a/src/platforms/android/__tests__/permission-grant-state.test.ts b/src/platforms/android/__tests__/permission-grant-state.test.ts new file mode 100644 index 000000000..257294c03 --- /dev/null +++ b/src/platforms/android/__tests__/permission-grant-state.test.ts @@ -0,0 +1,132 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { + androidPriorGrantState, + parseAndroidRuntimePermissionGrants, +} from '../permission-grant-state.ts'; + +// Captured from `adb shell dumpsys package com.callstack.agentdevicelab` on a Pixel 7 / API 36 +// emulator, trimmed to the sections that decide the answer. The indentation is load-bearing: +// `install permissions:` and each `User :` sit at the same depth, and the runtime grants +// hang under the user block — which is the only reason the two can be told apart. +const DUMPSYS = [ + 'Packages:', + ' Package [com.example.app] (5f3a1c2):', + ' userId=10234', + ' declared permissions:', + ' com.example.app.DYNAMIC_RECEIVER_NOT_EXPORTED_PERMISSION: prot=signature', + ' install permissions:', + ' android.permission.MODIFY_AUDIO_SETTINGS: granted=true', + ' android.permission.RECORD_AUDIO: granted=true', + ' User 0: ceDataInode=1032405 installed=true hidden=false stopped=false', + ' gids=[3003]', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=false, flags=[ USER_SENSITIVE_WHEN_GRANTED]', + ' android.permission.CAMERA: granted=true, flags=[ USER_SET]', + ' User 10: ceDataInode=99 installed=true hidden=false stopped=false', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET]', + '', + 'Queries:', + ' queryable via interaction:', + ' User 0:', + '', + 'Dexopt state:', + ' [com.example.app]', + ' path: /data/app/~~abc==/com.example.app-def==', +].join('\n'); + +test('runtime grants are read from the requested user only', () => { + const user0 = parseAndroidRuntimePermissionGrants(DUMPSYS, 0); + assert.equal(androidPriorGrantState(user0, 'android.permission.RECORD_AUDIO'), 'not_granted'); + assert.equal(androidPriorGrantState(user0, 'android.permission.CAMERA'), 'granted'); + + // The same permission, granted for another profile: `pm revoke` acts on the acting user, so + // reporting user 10's grant for a user 0 revoke would claim a kill that never happened. + const user10 = parseAndroidRuntimePermissionGrants(DUMPSYS, 10); + assert.equal(androidPriorGrantState(user10, 'android.permission.RECORD_AUDIO'), 'granted'); + assert.equal(androidPriorGrantState(user10, 'android.permission.CAMERA'), 'unknown'); +}); + +test('install permissions never answer for a runtime permission', () => { + // RECORD_AUDIO appears as `granted=true` in the install-permission section and `granted=false` + // in user 0's runtime block. A scan that matched `granted=true` anywhere read the wrong one. + const grants = parseAndroidRuntimePermissionGrants(DUMPSYS, 0); + assert.equal(androidPriorGrantState(grants, 'android.permission.RECORD_AUDIO'), 'not_granted'); + assert.equal( + androidPriorGrantState(grants, 'android.permission.MODIFY_AUDIO_SETTINGS'), + 'unknown', + ); + assert.equal(androidPriorGrantState(grants, 'android.permission.INTERNET'), 'unknown'); +}); + +test('a user with no runtime-permission block is unknown, not empty', () => { + assert.equal(parseAndroidRuntimePermissionGrants(DUMPSYS, 11), undefined); + assert.equal( + androidPriorGrantState( + parseAndroidRuntimePermissionGrants(DUMPSYS, 11), + 'android.permission.RECORD_AUDIO', + ), + 'unknown', + ); +}); + +test('unparseable output is unknown rather than not-granted', () => { + for (const output of ['', 'Packages:\n ', 'Error: package not found']) { + assert.equal(parseAndroidRuntimePermissionGrants(output, 0), undefined, output); + } +}); + +test('an empty runtime block answers not_granted for everything it could have listed', () => { + // Distinct from a missing block: the device reported the list and it was empty, so the app + // holds nothing — a claim the parser is allowed to make. + const grants = parseAndroidRuntimePermissionGrants( + [ + 'Packages:', + ' Package [com.example.app] (5f3a1c2):', + ' User 0: installed=true', + ' runtime permissions:', + ' User 10: installed=true', + ].join('\n'), + 0, + ); + assert.notEqual(grants, undefined); + assert.equal(grants?.size, 0); +}); + +test('a dump without a Packages section is unknown', () => { + // Every `dumpsys package ` carries one (it sits after the resolver tables and Key Set + // Manager). Its absence means the output is not the dump we can read, so the honest answer + // is unknown — never the not_granted that would claim the app was left alone. + const grants = parseAndroidRuntimePermissionGrants( + [ + 'Activity Resolver Table:', + ' Non-Data Actions:', + ' User 0: installed=true', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET]', + ].join('\n'), + 0, + ); + assert.equal(grants, undefined); +}); + +test('sections after Packages: cannot reopen the scan', () => { + // `Queries:` repeats `User 0:` with no grants, and `Shared users:` repeats runtime grant lines + // for the shared uid. Merging either would flip RECORD_AUDIO to granted for a user whose own + // block says otherwise — the same wrong-answer class as reading the install section. + const grants = parseAndroidRuntimePermissionGrants( + [ + DUMPSYS, + 'Shared users:', + ' SharedUser [android.uid.shared] (a1b2):', + ' User 0: ceDataInode=0 installed=true', + ' runtime permissions:', + ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET]', + ].join('\n'), + 0, + ); + + assert.equal(androidPriorGrantState(grants, 'android.permission.RECORD_AUDIO'), 'not_granted'); + assert.equal(androidPriorGrantState(grants, 'android.permission.CAMERA'), 'granted'); +}); diff --git a/src/platforms/android/__tests__/settings.test.ts b/src/platforms/android/__tests__/settings.test.ts index c08a514f2..4023a2a13 100644 --- a/src/platforms/android/__tests__/settings.test.ts +++ b/src/platforms/android/__tests__/settings.test.ts @@ -1,10 +1,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { - androidRevokedGrantedPermissionWarning, - parseAndroidGrantedRuntimePermissions, - setAndroidSetting, -} from '../settings.ts'; +import { androidRevokedPermissionWarning, setAndroidSetting } from '../settings.ts'; import { ANDROID_EMULATOR, assertRejectsAppError, @@ -181,67 +177,163 @@ test('setAndroidSetting permission reset notifications clears permission flags f ); }); -// #1796: Android kills the app when a permission it holds is revoked. The prior grant state is -// read before `pm revoke`, and a granted -> revoked transition surfaces as a warning + typed field. -const DUMPSYS_MICROPHONE_GRANTED = [ - 'Packages:', - ' Package [com.example.app] (abc):', - ' User 0: ceDataInode=0 installed=true', - ' runtime permissions:', - ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET|USER_SENSITIVE_WHEN_GRANTED]', - ' android.permission.CAMERA: granted=false, flags=[ USER_SENSITIVE_WHEN_GRANTED]', -].join('\n'); +// #1796: Android kills the app when a permission it holds is revoked. The acting user's prior +// state is read before `pm revoke`; the response reports it as a three-state typed field, because +// a failed or unreadable dump is NOT evidence that the app was left alone. +const CURRENT_USER = 'shell am get-current-user'; +const DUMPSYS = 'shell dumpsys package com.example.app'; +const REVOKE_MICROPHONE = 'shell pm revoke com.example.app android.permission.RECORD_AUDIO'; + +/** A dump shaped like the real one: an install-permission section, then per-user blocks. */ +function dumpsys( + users: ReadonlyArray<{ id: number; runtime?: ReadonlyArray<[string, boolean]> }>, +): string { + return [ + 'Packages:', + ' Package [com.example.app] (abc):', + ' install permissions:', + // Install permissions are granted for the package, not per user, and `pm revoke` cannot + // touch them — a scan that reads `granted=true` anywhere reports these as runtime grants. + ' android.permission.INTERNET: granted=true', + ' android.permission.RECORD_AUDIO: granted=true', + ...users.flatMap(({ id, runtime }) => [ + ` User ${id}: ceDataInode=0 installed=true`, + ...(runtime + ? [ + ' runtime permissions:', + ...runtime.map( + ([permission, granted]) => + ` ${permission}: granted=${granted}, flags=[ USER_SET]`, + ), + ] + : []), + ]), + // A later section repeats `User :` without any runtime block. + 'Queries:', + ' queryable via interaction:', + ' User 0:', + ].join('\n'); +} + +function fakeAdb( + script: (flat: string) => string | undefined | { stderr: string; exitCode: number }, +) { + return (args: string[]) => script(args.join(' ')); +} test.each(['deny', 'reset'] as const)( - 'setAndroidSetting permission %s warns that revoking a granted permission killed the app', + 'setAndroidSetting permission %s reports a granted prior state and warns', async (action) => { await withFakeAdb( - (args) => - args.join(' ') === 'shell dumpsys package com.example.app' - ? DUMPSYS_MICROPHONE_GRANTED - : undefined, + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) { + return dumpsys([{ id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }]); + } + return undefined; + }), async ({ calls, device }) => { const result = await setAndroidSetting(device, 'permission', action, 'com.example.app', { permissionTarget: 'microphone', }); const flat = calls.map((args) => args.join(' ')); // The state is read BEFORE the revoke: after it, dumpsys would already say false. - assert.ok( - flat.indexOf('shell dumpsys package com.example.app') < - flat.indexOf('shell pm revoke com.example.app android.permission.RECORD_AUDIO'), - flat.join('; '), - ); + assert.ok(flat.indexOf(DUMPSYS) < flat.indexOf(REVOKE_MICROPHONE), flat.join('; ')); assert.deepEqual(result, { permission: 'android.permission.RECORD_AUDIO', - wasGranted: true, + priorGrantState: 'granted', warnings: [ - androidRevokedGrantedPermissionWarning( + androidRevokedPermissionWarning( 'com.example.app', 'android.permission.RECORD_AUDIO', + 'granted', ), ], }); assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); - // The warning states the platform rule and keeps the consequence conditional: the - // prior-grant read proves neither that the app was running nor that the grant was - // the current user's (#1796 review). + // Conditional on purpose: the read proves neither that the app was running nor that a + // grant seen elsewhere belonged to the acting user (#1796 review). assert.match(String(result?.warnings), /if com\.example\.app was running it is no longer/); }, ); }, ); -test('setAndroidSetting permission deny stays quiet when the permission was not granted', async () => { +test('setAndroidSetting permission deny stays quiet when the acting user did not hold it', async () => { await withFakeAdb( - (args) => - args.join(' ') === 'shell dumpsys package com.example.app' - ? DUMPSYS_MICROPHONE_GRANTED - : undefined, + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) { + return dumpsys([{ id: 0, runtime: [['android.permission.CAMERA', false]] }]); + } + return undefined; + }), async ({ device }) => { const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { permissionTarget: 'camera', }); - assert.deepEqual(result, { permission: 'android.permission.CAMERA', wasGranted: false }); + assert.deepEqual(result, { + permission: 'android.permission.CAMERA', + priorGrantState: 'not_granted', + }); + }, + ); +}); + +// The three ways the prior state is genuinely unknown. Each must report `unknown` and still +// hand over the relaunch guidance: asserting `not_granted` here would claim the app survived. +test.each([ + ['dumpsys fails', (flat: string) => (flat === DUMPSYS ? { stderr: 'error', exitCode: 1 } : '0')], + [ + 'dumpsys output is unparseable', + (flat: string) => (flat === DUMPSYS ? 'Packages:\n ' : '0'), + ], + [ + 'the acting user cannot be resolved', + (flat: string) => + flat === CURRENT_USER + ? { stderr: 'cmd: not found', exitCode: 1 } + : dumpsys([{ id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }]), + ], +] as const)( + 'setAndroidSetting permission deny reports unknown (not not_granted) when %s', + async (_label, script) => { + await withFakeAdb( + fakeAdb((flat) => script(flat) as string | { stderr: string; exitCode: number }), + async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.equal(result?.priorGrantState, 'unknown'); + assert.match(String(result?.warnings), /could not be read/); + assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); + }, + ); + }, +); + +// A grant held by another profile is not this revoke's business: `pm revoke` acts on the +// acting user, so user 10's grant must not make user 0's revoke claim the app was killed. +test("setAndroidSetting permission deny reads only the acting user's block", async () => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) { + return dumpsys([ + { id: 0, runtime: [['android.permission.RECORD_AUDIO', false]] }, + { id: 10, runtime: [['android.permission.RECORD_AUDIO', true]] }, + ]); + } + return undefined; + }), + async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.deepEqual(result, { + permission: 'android.permission.RECORD_AUDIO', + priorGrantState: 'not_granted', + }); }, ); }); @@ -261,30 +353,25 @@ test('setAndroidSetting permission grant does not read grant state', async () => ); }); -test('setAndroidSetting permission reset notifications warns when POST_NOTIFICATIONS was granted', async () => { +test('setAndroidSetting permission reset notifications reports the POST_NOTIFICATIONS state', async () => { await withFakeAdb( - (args) => - args.join(' ') === 'shell dumpsys package com.example.app' - ? ' runtime permissions:\n android.permission.POST_NOTIFICATIONS: granted=true, flags=[ USER_SET]' - : undefined, + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) { + return dumpsys([{ id: 0, runtime: [['android.permission.POST_NOTIFICATIONS', true]] }]); + } + return undefined; + }), async ({ device }) => { const result = await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { permissionTarget: 'notifications', }); - assert.equal(result?.wasGranted, true); + assert.equal(result?.priorGrantState, 'granted'); assert.equal(Array.isArray(result?.warnings), true); }, ); }); -test('parseAndroidGrantedRuntimePermissions reads only granted=true runtime permissions', () => { - assert.deepEqual( - [...parseAndroidGrantedRuntimePermissions(DUMPSYS_MICROPHONE_GRANTED)], - ['android.permission.RECORD_AUDIO'], - ); - assert.deepEqual([...parseAndroidGrantedRuntimePermissions('')], []); -}); - test('setAndroidSetting permission reset camera clears permission flags for reprompt', async () => { await withFakeAdb( () => undefined, diff --git a/src/platforms/android/permission-grant-state.ts b/src/platforms/android/permission-grant-state.ts new file mode 100644 index 000000000..039b57b72 --- /dev/null +++ b/src/platforms/android/permission-grant-state.ts @@ -0,0 +1,128 @@ +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { runAndroidAdb } from './adb.ts'; + +/** + * Whether the app held a runtime permission immediately before a revoke. + * + * `unknown` is a first-class answer, not a synonym for `not_granted`: the state is read from + * `dumpsys package`, which can fail, be truncated, or not list the permission for the acting + * user at all. Collapsing that into `not_granted` would make the response assert the app was + * untouched exactly when we cannot tell — and the consequence of being wrong is an agent whose + * app Android silently killed (#1796). + */ +export type AndroidPriorGrantState = 'granted' | 'not_granted' | 'unknown'; + +/** + * The grants a revoke's prior state is resolved against. Held opaque so a caller reads the + * state once, before the revoke, and can still answer for a permission it only learns + * afterwards (`photos` resolves its permission by probing the device). + */ +export type AndroidRuntimePermissionGrants = ReadonlyMap; + +/** The state of one permission within a grants read; `unknown` when either is absent. */ +export function androidPriorGrantState( + grants: AndroidRuntimePermissionGrants | undefined, + permission: string, +): AndroidPriorGrantState { + return grants?.get(permission) ?? 'unknown'; +} + +/** + * The acting user's runtime permissions, or `undefined` when the state could not be read — + * adb failed, the current user could not be resolved, or the dump carried no runtime-permission + * block for that user. + * + * `pm revoke` without `--user` acts on the caller's user, so the state must be read from that + * same user's block. `dumpsys package` prints an `install permissions:` section and one block + * per user, all carrying `granted=` lines; a scan that ignores that structure reports another + * profile's grant — or an install permission that `pm revoke` cannot touch — as this user's. + */ +export async function readAndroidRuntimePermissionGrants( + device: DeviceInfo, + appPackage: string, +): Promise { + const userId = await readAndroidCurrentUserId(device); + if (userId === undefined) return undefined; + const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { + allowFailure: true, + }); + if (result.exitCode !== 0) return undefined; + return parseAndroidRuntimePermissionGrants(result.stdout, userId); +} + +/** The user `adb shell` commands act on, or `undefined` when it cannot be resolved. */ +async function readAndroidCurrentUserId(device: DeviceInfo): Promise { + const result = await runAndroidAdb(device, ['shell', 'am', 'get-current-user'], { + allowFailure: true, + }); + if (result.exitCode !== 0) return undefined; + const parsed = Number.parseInt(result.stdout.trim(), 10); + return Number.isInteger(parsed) && parsed >= 0 ? parsed : undefined; +} + +const TOP_LEVEL_SECTION = /^\S.*:\s*$/; +const PACKAGES_SECTION = 'Packages:'; +const USER_BLOCK = /^\s*User (\d+):/; +const RUNTIME_PERMISSIONS_BLOCK = /^\s*runtime permissions:\s*$/; +const GRANT_LINE = /^\s*([\w.]+): granted=(true|false)\b/; + +/** A non-blank dump line with the indentation that places it in the tree. */ +type DumpLine = { text: string; indent: number }; + +/** + * Runtime permission grants for `userId` only, or `undefined` when that user has no + * runtime-permission block in the dump. + * + * `dumpsys package` is indentation-structured: top-level `Packages:` holds the package's own + * blocks, `User :` opens a per-user block, and `runtime permissions:` opens the grant list + * inside it. The read walks that nesting instead of matching `granted=` anywhere, which is what + * keeps the `install permissions:` section, other users' blocks, and the later top-level + * sections (`Queries:`, `Shared users:`, `Dexopt state:` — which repeat `User :` and can + * repeat grant lines) out of the answer. + */ +export function parseAndroidRuntimePermissionGrants( + dumpsysOutput: string, + userId: number, +): AndroidRuntimePermissionGrants | undefined { + const packages = topLevelSection(readDumpLines(dumpsysOutput), PACKAGES_SECTION); + const user = nestedBlock(packages, (line) => USER_BLOCK.exec(line.text)?.[1] === String(userId)); + const runtime = nestedBlock(user, (line) => RUNTIME_PERMISSIONS_BLOCK.test(line.text)); + // Absent block: the device never reported this user's grants. An empty one is still an + // answer — the app holds no runtime permissions for this user. + if (!runtime) return undefined; + const grants = new Map(); + for (const { text } of runtime) { + const grant = GRANT_LINE.exec(text); + if (grant) grants.set(grant[1]!, grant[2] === 'true' ? 'granted' : 'not_granted'); + } + return grants; +} + +function readDumpLines(dumpsysOutput: string): DumpLine[] { + return dumpsysOutput + .split('\n') + .filter((text) => text.trim().length > 0) + .map((text) => ({ text, indent: text.length - text.trimStart().length })); +} + +/** The lines under a top-level `` header, up to the next top-level header. */ +function topLevelSection(lines: readonly DumpLine[], name: string): DumpLine[] { + const start = lines.findIndex((line) => line.indent === 0 && line.text.trim() === name); + if (start < 0) return []; + const rest = lines.slice(start + 1); + const end = rest.findIndex((line) => line.indent === 0 && TOP_LEVEL_SECTION.test(line.text)); + return end < 0 ? rest : rest.slice(0, end); +} + +/** The lines nested under the first line `isHeader` accepts, or `undefined` if there is none. */ +function nestedBlock( + lines: readonly DumpLine[] | undefined, + isHeader: (line: DumpLine) => boolean, +): DumpLine[] | undefined { + if (!lines) return undefined; + const start = lines.findIndex((line) => isHeader(line)); + if (start < 0) return undefined; + const rest = lines.slice(start + 1); + const end = rest.findIndex((line) => line.indent <= lines[start]!.indent); + return end < 0 ? rest : rest.slice(0, end); +} diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index 67b61afdd..a8688ad20 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -15,6 +15,11 @@ import { parseSettingState } from '../setting-state.ts'; import { runAndroidAdb } from './adb.ts'; import { androidAdbResultError } from './adb-executor.ts'; import { resolveAndroidApp } from './app-deployment-resolution.ts'; +import { + androidPriorGrantState, + readAndroidRuntimePermissionGrants, + type AndroidPriorGrantState, +} from './permission-grant-state.ts'; const ANDROID_ANIMATION_SCALE_SETTINGS = [ 'window_animation_scale', @@ -155,20 +160,22 @@ export async function setAndroidSetting( * follows a grant leaves the session pointing at a dead app and the next selector fails * against the launcher (#1796). Revoking a permission the app does not hold is harmless. * - * The prior grant state is read before the revoke; process death itself is NOT observed - * (that would be option (b) in the issue), and the read cannot prove the app was running - * or that the grant belonged to the current user profile. The wording therefore states the - * platform rule and makes the consequence conditional rather than asserting a death. + * Process death itself is NOT observed (that would be option (b) in the issue) and the prior + * state cannot prove the app was running, so the consequence stays conditional. When the state + * could not be read, the same guidance is given without claiming what the state was: silence + * there would assert "your app is untouched" on no evidence. */ -export function androidRevokedGrantedPermissionWarning( +export function androidRevokedPermissionWarning( appPackage: string, permission: string, -): string { - return ( - `${permission} was granted before this revoke, and Android kills an app when a granted ` + - `permission is revoked: if ${appPackage} was running it is no longer. Relaunch it with ` + - `open ${appPackage} --relaunch before the next interaction.` - ); + priorGrantState: AndroidPriorGrantState, +): string | undefined { + if (priorGrantState === 'not_granted') return undefined; + const preamble = + priorGrantState === 'granted' + ? `${permission} was granted before this revoke, and Android kills an app when a granted permission is revoked: if ${appPackage} was running it is no longer.` + : `Whether ${permission} was granted before this revoke could not be read (adb did not report the acting user's runtime permission state), and Android kills an app when a granted permission is revoked: ${appPackage} may no longer be running.`; + return `${preamble} Relaunch it with open ${appPackage} --relaunch before the next interaction.`; } type AndroidPermissionTarget = ReturnType; @@ -185,15 +192,16 @@ async function setAndroidPermission( await grantAndroidPermission(device, appPackage, target); return; } - const grantedBefore = await readAndroidGrantedRuntimePermissions(device, appPackage); + // Read before the revoke — afterwards every permission reads as not granted — but resolved + // after it, because `photos` only learns which permission it revoked by probing the device. + const grants = await readAndroidRuntimePermissionGrants(device, appPackage); const permission = await revokeAndroidPermission(device, appPackage, action, target); - const wasGranted = grantedBefore.has(permission); + const priorGrantState = androidPriorGrantState(grants, permission); + const warning = androidRevokedPermissionWarning(appPackage, permission, priorGrantState); return { permission, - wasGranted, - ...(wasGranted - ? { warnings: [androidRevokedGrantedPermissionWarning(appPackage, permission)] } - : {}), + priorGrantState, + ...(warning ? { warnings: [warning] } : {}), }; } @@ -233,29 +241,6 @@ async function revokeAndroidPermission( return permission; } -/** - * Runtime permissions `dumpsys package ` reports as `granted=true`. Best-effort: a - * failed or unparseable dump reads as "nothing granted", which only costs the warning. - */ -async function readAndroidGrantedRuntimePermissions( - device: DeviceInfo, - appPackage: string, -): Promise> { - const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { - allowFailure: true, - }); - if (result.exitCode !== 0) return new Set(); - return parseAndroidGrantedRuntimePermissions(result.stdout); -} - -export function parseAndroidGrantedRuntimePermissions(dumpsysOutput: string): ReadonlySet { - const granted = new Set(); - for (const match of dumpsysOutput.matchAll(/^\s*([\w.]+): granted=true\b/gm)) { - granted.add(match[1]!); - } - return granted; -} - type AndroidFingerprintAction = 'match' | 'nonmatch'; function parseAndroidFingerprintAction(state: string): AndroidFingerprintAction { diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index f8c31c8b1..0338458ea 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -674,7 +674,7 @@ agent-device settings permission reset screen-recording --platform macos - On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions. - On macOS, `settings permission deny ...` is intentionally unsupported. - Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. -- Android kills a running app whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app no longer running. The response then carries `wasGranted: true` and a warning; relaunch with `open --relaunch` before the next interaction. Revoking a permission the app does not hold is harmless. +- Android kills a running app whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app no longer running. The response reports the prior state of the revoked permission for the acting user as `priorGrantState: granted | not_granted | unknown`, and carries a warning naming `open --relaunch` for both `granted` and `unknown` — `unknown` means the device did not report a readable state, not that the app was left alone. Revoking a permission the app does not hold (`not_granted`) is harmless and warns nothing. - `full|limited` mode is supported only for iOS `photos`; other targets reject mode. - Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes. From 70aa573a3f4e216cfdfd161ea2b0e00308c92aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 14:32:01 +0200 Subject: [PATCH 4/8] fix(android): address the foreground user in every permission mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tri-state read scoped state to am get-current-user, but the mutations ran bare pm grant/revoke and clear-permission-flags. PackageManagerShellCommand defaults those to UserHandle.USER_SYSTEM, so on a device whose foreground user is nonzero the command read one user's state and edited user 0 — leaving the running app's permission untouched while reporting on a user it did not change. Proven on a Pixel 7 / API 36 emulator with the foreground user switched to 10: a bare pm revoke flipped User 0 to granted=false and left User 10 granted=true. The foreground user is now resolved once and passed as --user to pm grant/revoke, pm clear-permission-flags, and appops set, and the state read takes that same id. When it cannot be resolved the mutation keeps the platform default and the state is reported unknown rather than guessed. --- .../android/__tests__/settings.test.ts | 151 +++++++++++++++++- .../android/permission-grant-state.ts | 28 ++-- src/platforms/android/settings.ts | 94 +++++++---- website/docs/docs/commands.md | 2 +- 4 files changed, 229 insertions(+), 46 deletions(-) diff --git a/src/platforms/android/__tests__/settings.test.ts b/src/platforms/android/__tests__/settings.test.ts index 4023a2a13..c349c9665 100644 --- a/src/platforms/android/__tests__/settings.test.ts +++ b/src/platforms/android/__tests__/settings.test.ts @@ -182,7 +182,8 @@ test('setAndroidSetting permission reset notifications clears permission flags f // a failed or unreadable dump is NOT evidence that the app was left alone. const CURRENT_USER = 'shell am get-current-user'; const DUMPSYS = 'shell dumpsys package com.example.app'; -const REVOKE_MICROPHONE = 'shell pm revoke com.example.app android.permission.RECORD_AUDIO'; +const REVOKE_MICROPHONE = + 'shell pm revoke --user 0 com.example.app android.permission.RECORD_AUDIO'; /** A dump shaped like the real one: an install-permission section, then per-user blocks. */ function dumpsys( @@ -338,16 +339,160 @@ test("setAndroidSetting permission deny reads only the acting user's block", asy ); }); +// The read scopes state to the foreground user, so the mutation has to name the same one: +// PackageManagerShellCommand defaults grant/revoke/permission-flag operations to +// UserHandle.USER_SYSTEM, so a bare `pm revoke` on a device whose foreground user is 10 edits +// user 0 and leaves the running app untouched. Proven on a Pixel 7 / API 36 emulator with the +// foreground user switched to 10: bare `pm revoke` flipped User 0 to granted=false while +// User 10 stayed granted=true. These pin the exact argv and order, because a response-shape +// assertion passes either way. +test.each([ + [ + 'deny microphone', + { permissionTarget: 'microphone' } as const, + 'deny' as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + [ + 'shell', + 'pm', + 'revoke', + '--user', + '10', + 'com.example.app', + 'android.permission.RECORD_AUDIO', + ], + ], + ], + [ + 'reset camera', + { permissionTarget: 'camera' } as const, + 'reset' as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.CAMERA'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-set'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-fixed'], + ], + ], + [ + 'reset notifications', + { permissionTarget: 'notifications' } as const, + 'reset' as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + // prettier-ignore + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-set'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-fixed'], + // prettier-ignore + ['shell', 'appops', 'set', '--user', '10', 'com.example.app', 'POST_NOTIFICATION', 'default'], + ], + ], +] as const)( + 'setAndroidSetting permission %s addresses the foreground user in every adb call', + async (_label, options, action, expected) => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '10'; + if (flat === DUMPSYS) { + return dumpsys([ + { id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }, + { id: 10, runtime: [['android.permission.RECORD_AUDIO', false]] }, + ]); + } + return undefined; + }), + async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', action, 'com.example.app', options); + assert.deepEqual( + calls, + expected.map((args) => [...args]), + ); + }, + ); + }, +); + +test('setAndroidSetting permission grant addresses the foreground user too', async () => { + await withFakeAdb( + fakeAdb((flat) => (flat === CURRENT_USER ? '10' : undefined)), + async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.deepEqual(calls, [ + ['shell', 'am', 'get-current-user'], + // prettier-ignore + ['shell', 'pm', 'grant', '--user', '10', 'com.example.app', 'android.permission.RECORD_AUDIO'], + ]); + }, + ); +}); + +// The read and the mutation must agree about WHICH user, not merely both name one: with the +// foreground user 10 holding the permission and user 0 not, the response must report granted. +test('setAndroidSetting permission deny reads the same user it revokes', async () => { + await withFakeAdb( + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '10'; + if (flat === DUMPSYS) { + return dumpsys([ + { id: 0, runtime: [['android.permission.RECORD_AUDIO', false]] }, + { id: 10, runtime: [['android.permission.RECORD_AUDIO', true]] }, + ]); + } + return undefined; + }), + async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.equal(result?.priorGrantState, 'granted'); + assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); + }, + ); +}); + +// No resolvable user means no way to address one: the mutation keeps the platform default and +// the state is reported unknown rather than read from a user the revoke may not have touched. +test('setAndroidSetting permission deny omits --user when the foreground user is unknown', async () => { + await withFakeAdb( + fakeAdb((flat) => + flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : undefined, + ), + async ({ calls, device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.deepEqual(calls, [ + ['shell', 'am', 'get-current-user'], + ['shell', 'pm', 'revoke', 'com.example.app', 'android.permission.RECORD_AUDIO'], + ]); + assert.equal(result?.priorGrantState, 'unknown'); + }, + ); +}); + test('setAndroidSetting permission grant does not read grant state', async () => { await withFakeAdb( - () => undefined, + fakeAdb((flat) => (flat === CURRENT_USER ? '0' : undefined)), async ({ calls, device }) => { const result = await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { permissionTarget: 'camera', }); assert.equal(result, undefined); + // The user is resolved for the mutation, but no state is read: grant cannot kill the app. assert.deepEqual(calls, [ - ['shell', 'pm', 'grant', 'com.example.app', 'android.permission.CAMERA'], + ['shell', 'am', 'get-current-user'], + ['shell', 'pm', 'grant', '--user', '0', 'com.example.app', 'android.permission.CAMERA'], ]); }, ); diff --git a/src/platforms/android/permission-grant-state.ts b/src/platforms/android/permission-grant-state.ts index 039b57b72..d63b75b21 100644 --- a/src/platforms/android/permission-grant-state.ts +++ b/src/platforms/android/permission-grant-state.ts @@ -28,21 +28,19 @@ export function androidPriorGrantState( } /** - * The acting user's runtime permissions, or `undefined` when the state could not be read — - * adb failed, the current user could not be resolved, or the dump carried no runtime-permission - * block for that user. + * `userId`'s runtime permissions, or `undefined` when the state could not be read — adb failed + * or the dump carried no runtime-permission block for that user. * - * `pm revoke` without `--user` acts on the caller's user, so the state must be read from that - * same user's block. `dumpsys package` prints an `install permissions:` section and one block - * per user, all carrying `granted=` lines; a scan that ignores that structure reports another - * profile's grant — or an install permission that `pm revoke` cannot touch — as this user's. + * The caller passes the user its mutation will target, so the two halves cannot disagree. + * `dumpsys package` prints an `install permissions:` section and one block per user, all + * carrying `granted=` lines; a scan that ignores that structure reports another profile's + * grant — or an install permission that `pm revoke` cannot touch — as this user's. */ export async function readAndroidRuntimePermissionGrants( device: DeviceInfo, appPackage: string, + userId: number, ): Promise { - const userId = await readAndroidCurrentUserId(device); - if (userId === undefined) return undefined; const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { allowFailure: true, }); @@ -50,8 +48,16 @@ export async function readAndroidRuntimePermissionGrants( return parseAndroidRuntimePermissionGrants(result.stdout, userId); } -/** The user `adb shell` commands act on, or `undefined` when it cannot be resolved. */ -async function readAndroidCurrentUserId(device: DeviceInfo): Promise { +/** + * The foreground user, or `undefined` when it cannot be resolved. + * + * This is the user the session's app runs as, and it is NOT what `pm` defaults to: + * `PackageManagerShellCommand` defaults grant/revoke/permission-flag operations to + * `UserHandle.USER_SYSTEM`, so on a device whose foreground user is nonzero a bare `pm revoke` + * silently edits user 0 and leaves the running app's permission untouched. Every permission + * mutation therefore passes `--user` explicitly (#1796). + */ +export async function readAndroidCurrentUserId(device: DeviceInfo): Promise { const result = await runAndroidAdb(device, ['shell', 'am', 'get-current-user'], { allowFailure: true, }); diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index a8688ad20..d04ac4657 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -17,6 +17,7 @@ import { androidAdbResultError } from './adb-executor.ts'; import { resolveAndroidApp } from './app-deployment-resolution.ts'; import { androidPriorGrantState, + readAndroidCurrentUserId, readAndroidRuntimePermissionGrants, type AndroidPriorGrantState, } from './permission-grant-state.ts'; @@ -180,6 +181,14 @@ export function androidRevokedPermissionWarning( type AndroidPermissionTarget = ReturnType; +/** + * `--user ` for every permission mutation, resolved once so the state read and the mutation + * cannot address different users. Empty only when the foreground user could not be resolved, in + * which case the platform default (`UserHandle.USER_SYSTEM`) applies and the prior state is + * reported as unknown rather than guessed. + */ +type AndroidUserArgs = readonly string[]; + async function setAndroidPermission( device: DeviceInfo, appPackage: string, @@ -188,14 +197,19 @@ async function setAndroidPermission( ): Promise | void> { const action = parsePermissionAction(state); const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); + const userId = await readAndroidCurrentUserId(device); + const userArgs: AndroidUserArgs = userId === undefined ? [] : ['--user', String(userId)]; if (action === 'grant') { - await grantAndroidPermission(device, appPackage, target); + await grantAndroidPermission(device, appPackage, target, userArgs); return; } // Read before the revoke — afterwards every permission reads as not granted — but resolved // after it, because `photos` only learns which permission it revoked by probing the device. - const grants = await readAndroidRuntimePermissionGrants(device, appPackage); - const permission = await revokeAndroidPermission(device, appPackage, action, target); + const grants = + userId === undefined + ? undefined + : await readAndroidRuntimePermissionGrants(device, appPackage, userId); + const permission = await revokeAndroidPermission(device, appPackage, action, target, userArgs); const priorGrantState = androidPriorGrantState(grants, permission); const warning = androidRevokedPermissionWarning(appPackage, permission, priorGrantState); return { @@ -209,13 +223,14 @@ async function grantAndroidPermission( device: DeviceInfo, appPackage: string, target: AndroidPermissionTarget, + userArgs: AndroidUserArgs, ): Promise { if (target.kind === 'notifications') { - await setAndroidNotificationPermission(device, appPackage, 'grant', target); + await setAndroidNotificationPermission(device, appPackage, 'grant', target, userArgs); } else if (target.type === 'photos') { - await setAndroidPhotoPermission(device, appPackage, 'grant'); + await setAndroidPhotoPermission(device, appPackage, 'grant', userArgs); } else { - await runAndroidAdb(device, ['shell', 'pm', 'grant', appPackage, target.value]); + await runAndroidAdb(device, ['shell', 'pm', 'grant', ...userArgs, appPackage, target.value]); } } @@ -225,19 +240,22 @@ async function revokeAndroidPermission( appPackage: string, action: 'deny' | 'reset', target: AndroidPermissionTarget, + userArgs: AndroidUserArgs, ): Promise { if (target.kind === 'notifications') { - await setAndroidNotificationPermission(device, appPackage, action, target); + await setAndroidNotificationPermission(device, appPackage, action, target, userArgs); return target.permission; } let permission: string; if (target.type === 'photos') { - permission = await setAndroidPhotoPermission(device, appPackage, 'revoke'); + permission = await setAndroidPhotoPermission(device, appPackage, 'revoke', userArgs); } else { permission = target.value; - await runAndroidAdb(device, ['shell', 'pm', 'revoke', appPackage, permission]); + await runAndroidAdb(device, ['shell', 'pm', 'revoke', ...userArgs, appPackage, permission]); + } + if (action === 'reset') { + await clearAndroidPermissionFlags(device, appPackage, permission, userArgs); } - if (action === 'reset') await clearAndroidPermissionFlags(device, appPackage, permission); return permission; } @@ -400,6 +418,7 @@ async function setAndroidPhotoPermission( device: DeviceInfo, appPackage: string, pmAction: 'grant' | 'revoke', + userArgs: AndroidUserArgs, ): Promise { const sdkInt = await getAndroidSdkInt(device); const candidates = @@ -409,9 +428,11 @@ async function setAndroidPhotoPermission( const failures: Array<{ permission: string; stderr: string; exitCode: number }> = []; for (const permission of candidates) { - const result = await runAndroidAdb(device, ['shell', 'pm', pmAction, appPackage, permission], { - allowFailure: true, - }); + const result = await runAndroidAdb( + device, + ['shell', 'pm', pmAction, ...userArgs, appPackage, permission], + { allowFailure: true }, + ); if (result.exitCode === 0) return permission; failures.push({ permission, stderr: result.stderr, exitCode: result.exitCode }); } @@ -428,38 +449,49 @@ async function setAndroidNotificationPermission( appPackage: string, action: 'grant' | 'deny' | 'reset', target: { appOps: string; permission: string }, + userArgs: AndroidUserArgs, ): Promise { const appOpsMode = action === 'grant' ? 'allow' : action === 'deny' ? 'deny' : 'default'; if (action === 'grant') { - await runAndroidAdb(device, ['shell', 'pm', 'grant', appPackage, target.permission], { - allowFailure: true, - }); + await runAndroidAdb( + device, + ['shell', 'pm', 'grant', ...userArgs, appPackage, target.permission], + { allowFailure: true }, + ); } else { - await runAndroidAdb(device, ['shell', 'pm', 'revoke', appPackage, target.permission], { - allowFailure: true, - }); + await runAndroidAdb( + device, + ['shell', 'pm', 'revoke', ...userArgs, appPackage, target.permission], + { allowFailure: true }, + ); if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, target.permission); + await clearAndroidPermissionFlags(device, appPackage, target.permission, userArgs); } } - await runAndroidAdb(device, ['shell', 'appops', 'set', appPackage, target.appOps, appOpsMode]); + await runAndroidAdb(device, [ + 'shell', + 'appops', + 'set', + ...userArgs, + appPackage, + target.appOps, + appOpsMode, + ]); } async function clearAndroidPermissionFlags( device: DeviceInfo, appPackage: string, permission: string, + userArgs: AndroidUserArgs, ): Promise { - await runAndroidAdb( - device, - ['shell', 'pm', 'clear-permission-flags', appPackage, permission, 'user-set'], - { allowFailure: true }, - ); - await runAndroidAdb( - device, - ['shell', 'pm', 'clear-permission-flags', appPackage, permission, 'user-fixed'], - { allowFailure: true }, - ); + for (const flag of ['user-set', 'user-fixed']) { + await runAndroidAdb( + device, + ['shell', 'pm', 'clear-permission-flags', ...userArgs, appPackage, permission, flag], + { allowFailure: true }, + ); + } } async function getAndroidSdkInt(device: DeviceInfo): Promise { diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 0338458ea..b1dc990f5 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -673,7 +673,7 @@ agent-device settings permission reset screen-recording --platform macos - macOS permission targets: `accessibility`, `screen-recording`, `input-monitoring`. - On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions. - On macOS, `settings permission deny ...` is intentionally unsupported. -- Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. +- Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. Every permission mutation names the foreground user explicitly (`--user `, resolved with `am get-current-user`): `pm` defaults these operations to user 0, so on a device whose foreground user is nonzero an unscoped revoke would edit user 0 and leave the running app's permission untouched. - Android kills a running app whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app no longer running. The response reports the prior state of the revoked permission for the acting user as `priorGrantState: granted | not_granted | unknown`, and carries a warning naming `open --relaunch` for both `granted` and `unknown` — `unknown` means the device did not report a readable state, not that the app was left alone. Revoking a permission the app does not hold (`not_granted`) is harmless and warns nothing. - `full|limited` mode is supported only for iOS `photos`; other targets reject mode. - Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes. From adc8968fd6b1d647d1786a239f415b336541cb3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 14:40:11 +0200 Subject: [PATCH 5/8] test(android): pin the user-scoped permission argv in the provider scenario The scripted ADB provider answered only the unscoped pm grant/revoke form, and the Settings contract asserted the unscoped transcript entry, so the provider lane could not see which user a permission mutation addressed. --- .../provider-scenarios/android-lifecycle.test.ts | 5 +++++ test/integration/provider-scenarios/android-world.ts | 12 ++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 2e14f45da..82ee72099 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -1491,10 +1491,15 @@ function assertAndroidSettingsContract(world: AndroidSettingsWorld): void { assertCommandCall(adbCalls, ['shell', 'cmd', 'uimode', 'night', 'yes']); assertCommandCall(adbCalls, ['emu', 'geo', 'fix', '-122.009', '37.3349']); assertCommandCall(adbCalls, ['shell', 'cmd', 'fingerprint', 'touch', '1']); + // #1796: the grant names the acting user, because `pm` would otherwise default to user 0 + // rather than the foreground user the session's app runs as. + assertCommandCall(adbCalls, ['shell', 'am', 'get-current-user']); assertCommandCall(adbCalls, [ 'shell', 'pm', 'grant', + '--user', + '0', 'com.example.demo', 'android.permission.CAMERA', ]); diff --git a/test/integration/provider-scenarios/android-world.ts b/test/integration/provider-scenarios/android-world.ts index 5abda4f2b..5502f1d48 100644 --- a/test/integration/provider-scenarios/android-world.ts +++ b/test/integration/provider-scenarios/android-world.ts @@ -466,10 +466,18 @@ function androidFingerprintMutationAdbResult(args: string[]): AndroidAdbResult | } function androidPermissionMutationAdbResult(args: string[]): AndroidAdbResult | undefined { + // #1796: permission mutations name the acting user explicitly, because `pm` defaults + // grant/revoke to user 0 rather than the foreground user. The scripted provider answers the + // resolution and accepts the `--user ` form the production path now sends. + if (args.length === 3 && argsStartWith(args, ['shell', 'am', 'get-current-user'])) { + return { stdout: '0\n', stderr: '', exitCode: 0 }; + } + const scoped = args[3] === '--user'; + const verb = args[2]; if ( - args.length === 5 && argsStartWith(args, ['shell', 'pm']) && - (args[2] === 'grant' || args[2] === 'revoke') + (verb === 'grant' || verb === 'revoke') && + args.length === (scoped ? 7 : 5) ) { return { stdout: '', stderr: '', exitCode: 0 }; } From 169b77851e16627077df27b143d20088a9767a8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 14:56:20 +0200 Subject: [PATCH 6/8] test(android): extract the settings contract out of the lifecycle monolith The user-scoped argv assertions pushed android-lifecycle.test.ts past its size ratchet, whose instruction is to extract rather than grow a file over the tripwire. assertAndroidSettingsContract moves to a sibling module and the pin drops 1597 -> 1559. --- src/__tests__/test-file-size-ratchet.test.ts | 2 +- .../android-lifecycle.test.ts | 45 +--------------- .../android-settings-contract.ts | 54 +++++++++++++++++++ 3 files changed, 56 insertions(+), 45 deletions(-) create mode 100644 test/integration/provider-scenarios/android-settings-contract.ts diff --git a/src/__tests__/test-file-size-ratchet.test.ts b/src/__tests__/test-file-size-ratchet.test.ts index 92b77c979..54a3a6b88 100644 --- a/src/__tests__/test-file-size-ratchet.test.ts +++ b/src/__tests__/test-file-size-ratchet.test.ts @@ -43,7 +43,7 @@ const PINNED_TEST_FILE_LINES: Readonly> = Object.freeze({ 'src/platforms/android/__tests__/snapshot.test.ts': 1660, 'src/platforms/apple/core/__tests__/runner-client.test.ts': 1615, 'src/__tests__/client.test.ts': 1598, - 'test/integration/provider-scenarios/android-lifecycle.test.ts': 1597, + 'test/integration/provider-scenarios/android-lifecycle.test.ts': 1559, 'src/utils/__tests__/daemon-client-lifecycle.test.ts': 1414, 'src/platforms/apple/core/__tests__/runner-command-retry.test.ts': 1327, 'src/__tests__/cli-client-commands.test.ts': 1317, diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index 82ee72099..345c6b930 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -11,6 +11,7 @@ import { assertRpcOk, } from './assertions.ts'; import { createAndroidSettingsWorld, waitForFileContent } from './android-world.ts'; +import { assertAndroidSettingsContract } from './android-settings-contract.ts'; import { PROVIDER_SCENARIO_ANDROID } from './fixtures.ts'; import { createProviderScenarioTempPath, withProviderScenarioResource } from './harness.ts'; import { @@ -1486,50 +1487,6 @@ function assertAndroidObservabilityContract(world: AndroidSettingsWorld): void { ); } -function assertAndroidSettingsContract(world: AndroidSettingsWorld): void { - const { adbCalls } = world; - assertCommandCall(adbCalls, ['shell', 'cmd', 'uimode', 'night', 'yes']); - assertCommandCall(adbCalls, ['emu', 'geo', 'fix', '-122.009', '37.3349']); - assertCommandCall(adbCalls, ['shell', 'cmd', 'fingerprint', 'touch', '1']); - // #1796: the grant names the acting user, because `pm` would otherwise default to user 0 - // rather than the foreground user the session's app runs as. - assertCommandCall(adbCalls, ['shell', 'am', 'get-current-user']); - assertCommandCall(adbCalls, [ - 'shell', - 'pm', - 'grant', - '--user', - '0', - 'com.example.demo', - 'android.permission.CAMERA', - ]); - assertCommandCall(adbCalls, [ - 'shell', - 'settings', - 'put', - 'global', - 'window_animation_scale', - '0', - ]); - assertCommandCall(adbCalls, [ - 'shell', - 'settings', - 'put', - 'global', - 'transition_animation_scale', - '0', - ]); - assertCommandCall(adbCalls, [ - 'shell', - 'settings', - 'put', - 'global', - 'animator_duration_scale', - '0', - ]); - assertCommandCall(adbCalls, ['shell', 'echo', 'ok']); -} - function assertAndroidInteractionContract(world: AndroidSettingsWorld): void { const { adbCalls } = world; assert.ok( diff --git a/test/integration/provider-scenarios/android-settings-contract.ts b/test/integration/provider-scenarios/android-settings-contract.ts new file mode 100644 index 000000000..c0f4b2004 --- /dev/null +++ b/test/integration/provider-scenarios/android-settings-contract.ts @@ -0,0 +1,54 @@ +import { assertCommandCall } from './assertions.ts'; +import type { createAndroidSettingsWorld } from './android-world.ts'; + +type AndroidSettingsWorld = Awaited>; + +/** + * The adb evidence the Android settings flow must leave behind: the appearance, location, + * fingerprint, permission, animation-scale, and network mutations each named with the exact + * argv the provider saw. Extracted from `android-lifecycle.test.ts`, which sits over the + * test-file tripwire (`src/__tests__/test-file-size-ratchet.test.ts`). + */ +export function assertAndroidSettingsContract(world: AndroidSettingsWorld): void { + const { adbCalls } = world; + assertCommandCall(adbCalls, ['shell', 'cmd', 'uimode', 'night', 'yes']); + assertCommandCall(adbCalls, ['emu', 'geo', 'fix', '-122.009', '37.3349']); + assertCommandCall(adbCalls, ['shell', 'cmd', 'fingerprint', 'touch', '1']); + // #1796: the grant names the acting user, because `pm` would otherwise default to user 0 + // rather than the foreground user the session's app runs as. + assertCommandCall(adbCalls, ['shell', 'am', 'get-current-user']); + assertCommandCall(adbCalls, [ + 'shell', + 'pm', + 'grant', + '--user', + '0', + 'com.example.demo', + 'android.permission.CAMERA', + ]); + assertCommandCall(adbCalls, [ + 'shell', + 'settings', + 'put', + 'global', + 'window_animation_scale', + '0', + ]); + assertCommandCall(adbCalls, [ + 'shell', + 'settings', + 'put', + 'global', + 'transition_animation_scale', + '0', + ]); + assertCommandCall(adbCalls, [ + 'shell', + 'settings', + 'put', + 'global', + 'animator_duration_scale', + '0', + ]); + assertCommandCall(adbCalls, ['shell', 'echo', 'ok']); +} From fd39b4452164e24c1d487d18cde3f76c4a016924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 15:15:09 +0200 Subject: [PATCH 7/8] refactor(android): shrink the permission path to one concept per file Size/design pass on the #1796 change: - settings.ts was 505 lines (past the 500 extract-before-adding tripwire); the permission family moves to settings-permission.ts and the dispatcher drops to 265. - permission-grant-state.ts loses topLevelSection (a nestedBlock with an indent-0 header), its single-use line reader, and androidPriorGrantState (one map lookup at its only production call site). - the grants map narrows to 'granted' | 'not_granted': unknown was never a value, absence is what carries it, so the tests read the map directly. - the permission tests move to settings-permission.test.ts and consolidate into argv/tri-state/photos/rejection tables; the parser tests fold seven cases into two. Every red-proof re-run after the consolidation: dropping --user reds 7 argv/photos cases, and the pre-fix state model reds 13 across both files. --- .../__tests__/permission-grant-state.test.ts | 87 +-- .../__tests__/settings-permission.test.ts | 321 ++++++++++++ .../android/__tests__/settings.test.ts | 495 +----------------- .../android/permission-grant-state.ts | 70 +-- src/platforms/android/settings-permission.ts | 238 +++++++++ src/platforms/android/settings.ts | 244 +-------- 6 files changed, 614 insertions(+), 841 deletions(-) create mode 100644 src/platforms/android/__tests__/settings-permission.test.ts create mode 100644 src/platforms/android/settings-permission.ts diff --git a/src/platforms/android/__tests__/permission-grant-state.test.ts b/src/platforms/android/__tests__/permission-grant-state.test.ts index 257294c03..41fcb9c1c 100644 --- a/src/platforms/android/__tests__/permission-grant-state.test.ts +++ b/src/platforms/android/__tests__/permission-grant-state.test.ts @@ -1,9 +1,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { - androidPriorGrantState, - parseAndroidRuntimePermissionGrants, -} from '../permission-grant-state.ts'; +import { parseAndroidRuntimePermissionGrants } from '../permission-grant-state.ts'; // Captured from `adb shell dumpsys package com.callstack.agentdevicelab` on a Pixel 7 / API 36 // emulator, trimmed to the sections that decide the answer. The indentation is load-bearing: @@ -12,9 +9,6 @@ import { const DUMPSYS = [ 'Packages:', ' Package [com.example.app] (5f3a1c2):', - ' userId=10234', - ' declared permissions:', - ' com.example.app.DYNAMIC_RECEIVER_NOT_EXPORTED_PERMISSION: prot=signature', ' install permissions:', ' android.permission.MODIFY_AUDIO_SETTINGS: granted=true', ' android.permission.RECORD_AUDIO: granted=true', @@ -36,45 +30,29 @@ const DUMPSYS = [ ' path: /data/app/~~abc==/com.example.app-def==', ].join('\n'); -test('runtime grants are read from the requested user only', () => { - const user0 = parseAndroidRuntimePermissionGrants(DUMPSYS, 0); - assert.equal(androidPriorGrantState(user0, 'android.permission.RECORD_AUDIO'), 'not_granted'); - assert.equal(androidPriorGrantState(user0, 'android.permission.CAMERA'), 'granted'); - - // The same permission, granted for another profile: `pm revoke` acts on the acting user, so - // reporting user 10's grant for a user 0 revoke would claim a kill that never happened. - const user10 = parseAndroidRuntimePermissionGrants(DUMPSYS, 10); - assert.equal(androidPriorGrantState(user10, 'android.permission.RECORD_AUDIO'), 'granted'); - assert.equal(androidPriorGrantState(user10, 'android.permission.CAMERA'), 'unknown'); +// `undefined` is the answer for anything the dump does not place inside the requested user's +// runtime block: an install-permission grant (`pm revoke` cannot touch it), another profile's +// grant, or a permission that user never declared. The caller reports that as `unknown`. +test.each([ + [0, 'android.permission.RECORD_AUDIO', 'not_granted', 'acting user overrides the install grant'], + [0, 'android.permission.CAMERA', 'granted', 'acting user'], + [10, 'android.permission.RECORD_AUDIO', 'granted', 'other profile, only when asked for'], + [10, 'android.permission.CAMERA', undefined, 'absent for that user'], + [0, 'android.permission.MODIFY_AUDIO_SETTINGS', undefined, 'install-only'], + [0, 'android.permission.INTERNET', undefined, 'never mentioned'], +] as const)('user %s reads %s as %s (%s)', (userId, permission, expected, _why) => { + assert.equal(parseAndroidRuntimePermissionGrants(DUMPSYS, userId)?.get(permission), expected); }); -test('install permissions never answer for a runtime permission', () => { - // RECORD_AUDIO appears as `granted=true` in the install-permission section and `granted=false` - // in user 0's runtime block. A scan that matched `granted=true` anywhere read the wrong one. - const grants = parseAndroidRuntimePermissionGrants(DUMPSYS, 0); - assert.equal(androidPriorGrantState(grants, 'android.permission.RECORD_AUDIO'), 'not_granted'); - assert.equal( - androidPriorGrantState(grants, 'android.permission.MODIFY_AUDIO_SETTINGS'), - 'unknown', - ); - assert.equal(androidPriorGrantState(grants, 'android.permission.INTERNET'), 'unknown'); -}); - -test('a user with no runtime-permission block is unknown, not empty', () => { - assert.equal(parseAndroidRuntimePermissionGrants(DUMPSYS, 11), undefined); - assert.equal( - androidPriorGrantState( - parseAndroidRuntimePermissionGrants(DUMPSYS, 11), - 'android.permission.RECORD_AUDIO', - ), - 'unknown', - ); -}); - -test('unparseable output is unknown rather than not-granted', () => { - for (const output of ['', 'Packages:\n ', 'Error: package not found']) { - assert.equal(parseAndroidRuntimePermissionGrants(output, 0), undefined, output); - } +// A missing block is not an empty one. Every input here means "the device did not tell us", +// which must stay distinguishable from "the app holds nothing". +test.each([ + ['no runtime block for that user', DUMPSYS, 11], + ['empty output', '', 0], + ['no Packages section', 'Activity Resolver Table:\n User 0:\n runtime permissions:', 0], + ['a package section without the user', 'Packages:\n Package [com.example.app] (abc):', 0], +] as const)('%s reads as unknown', (_label, output, userId) => { + assert.equal(parseAndroidRuntimePermissionGrants(output, userId), undefined); }); test('an empty runtime block answers not_granted for everything it could have listed', () => { @@ -94,23 +72,6 @@ test('an empty runtime block answers not_granted for everything it could have li assert.equal(grants?.size, 0); }); -test('a dump without a Packages section is unknown', () => { - // Every `dumpsys package ` carries one (it sits after the resolver tables and Key Set - // Manager). Its absence means the output is not the dump we can read, so the honest answer - // is unknown — never the not_granted that would claim the app was left alone. - const grants = parseAndroidRuntimePermissionGrants( - [ - 'Activity Resolver Table:', - ' Non-Data Actions:', - ' User 0: installed=true', - ' runtime permissions:', - ' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET]', - ].join('\n'), - 0, - ); - assert.equal(grants, undefined); -}); - test('sections after Packages: cannot reopen the scan', () => { // `Queries:` repeats `User 0:` with no grants, and `Shared users:` repeats runtime grant lines // for the shared uid. Merging either would flip RECORD_AUDIO to granted for a user whose own @@ -127,6 +88,6 @@ test('sections after Packages: cannot reopen the scan', () => { 0, ); - assert.equal(androidPriorGrantState(grants, 'android.permission.RECORD_AUDIO'), 'not_granted'); - assert.equal(androidPriorGrantState(grants, 'android.permission.CAMERA'), 'granted'); + assert.equal(grants?.get('android.permission.RECORD_AUDIO'), 'not_granted'); + assert.equal(grants?.get('android.permission.CAMERA'), 'granted'); }); diff --git a/src/platforms/android/__tests__/settings-permission.test.ts b/src/platforms/android/__tests__/settings-permission.test.ts new file mode 100644 index 000000000..a48f49535 --- /dev/null +++ b/src/platforms/android/__tests__/settings-permission.test.ts @@ -0,0 +1,321 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { setAndroidSetting } from '../settings.ts'; +import { androidRevokedPermissionWarning } from '../settings-permission.ts'; +import { + ANDROID_EMULATOR, + assertRejectsAppError, + withFakeAdb, +} from '../../../__tests__/test-utils/index.ts'; + +// #1796. Two invariants decide every case here: +// * `pm` defaults grant/revoke and the permission-flag operations to UserHandle.USER_SYSTEM, +// so a bare mutation edits user 0 rather than the foreground user the session app runs as. +// Proven on a Pixel 7 / API 36 emulator with the foreground user switched to 10: a bare +// `pm revoke` flipped User 0 to granted=false while User 10 stayed granted=true. +// * The prior state is read for that same user, and a state we could not read is `unknown` — +// never `not_granted`, which would claim the app was left alone. +// The fake adb provider installs through the production withAndroidAdbProvider scope, so +// `calls` records device-scoped args without a leading `-s `. +const CURRENT_USER = 'shell am get-current-user'; +const DUMPSYS = 'shell dumpsys package com.example.app'; +const MICROPHONE = 'android.permission.RECORD_AUDIO'; + +/** A dump shaped like the real one: an install-permission section, then per-user blocks. */ +function dumpsys( + users: ReadonlyArray<{ id: number; runtime?: ReadonlyArray<[string, boolean]> }>, +): string { + return [ + 'Packages:', + ' Package [com.example.app] (abc):', + ' install permissions:', + // Install permissions are granted for the package, not per user, and `pm revoke` cannot + // touch them — a scan that reads `granted=true` anywhere reports these as runtime grants. + ' android.permission.INTERNET: granted=true', + ` ${MICROPHONE}: granted=true`, + ...users.flatMap(({ id, runtime }) => [ + ` User ${id}: ceDataInode=0 installed=true`, + ...(runtime + ? [ + ' runtime permissions:', + ...runtime.map(([permission, granted]) => ` ${permission}: granted=${granted}`), + ] + : []), + ]), + // A later top-level section repeats `User :` without any runtime block. + 'Queries:', + ' queryable via interaction:', + ' User 0:', + ].join('\n'); +} + +type FakeAdbReply = string | undefined | { stderr: string; exitCode: number }; + +function fakeAdb(script: (flat: string) => FakeAdbReply) { + return (args: string[]) => script(args.join(' ')); +} + +/** Foreground user `userId`, holding exactly `granted` of the microphone permission. */ +function foregroundUser(userId: string, granted: boolean) { + return fakeAdb((flat) => { + if (flat === CURRENT_USER) return userId; + if (flat === DUMPSYS) + return dumpsys([{ id: Number(userId), runtime: [[MICROPHONE, granted]] }]); + return undefined; + }); +} + +// Exact argv and order, on a device whose foreground user is NOT 0: a response-shape assertion +// passes whether or not the mutation named a user, so these pin the wire instead. +test.each([ + [ + 'deny microphone', + 'deny' as const, + { permissionTarget: 'microphone' } as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', MICROPHONE], + ], + ], + [ + 'reset camera', + 'reset' as const, + { permissionTarget: 'camera' } as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.CAMERA'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-set'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-fixed'], + ], + ], + [ + 'reset notifications', + 'reset' as const, + { permissionTarget: 'notifications' } as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + // prettier-ignore + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-set'], + // prettier-ignore + ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-fixed'], + // prettier-ignore + ['shell', 'appops', 'set', '--user', '10', 'com.example.app', 'POST_NOTIFICATION', 'default'], + ], + ], + [ + 'deny notifications', + 'deny' as const, + { permissionTarget: 'notifications' } as const, + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'dumpsys', 'package', 'com.example.app'], + // prettier-ignore + ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS'], + ['shell', 'appops', 'set', '--user', '10', 'com.example.app', 'POST_NOTIFICATION', 'deny'], + ], + ], + [ + 'grant microphone', + 'grant' as const, + { permissionTarget: 'microphone' } as const, + // No dumpsys: a grant cannot kill the app, so it reads no state. + [ + ['shell', 'am', 'get-current-user'], + ['shell', 'pm', 'grant', '--user', '10', 'com.example.app', MICROPHONE], + ], + ], +] as const)( + 'setAndroidSetting permission %s addresses the foreground user in every adb call', + async (_label, action, options, expected) => { + await withFakeAdb(foregroundUser('10', false), async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', action, 'com.example.app', options); + assert.deepEqual( + calls, + expected.map((args) => [...args]), + ); + }); + }, +); + +// The tri-state, including which user answers it. `unknown` must never be reported as +// `not_granted`, and must still hand over the relaunch guidance. +test.each([ + ['the acting user holds it', foregroundUser('0', true), 'granted'], + ['the acting user does not', foregroundUser('0', false), 'not_granted'], + [ + 'only another profile holds it', + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) { + return dumpsys([ + { id: 0, runtime: [[MICROPHONE, false]] }, + { id: 10, runtime: [[MICROPHONE, true]] }, + ]); + } + return undefined; + }), + 'not_granted', + ], + [ + 'the acting user is the one that holds it', + fakeAdb((flat) => { + if (flat === CURRENT_USER) return '10'; + if (flat === DUMPSYS) { + return dumpsys([ + { id: 0, runtime: [[MICROPHONE, false]] }, + { id: 10, runtime: [[MICROPHONE, true]] }, + ]); + } + return undefined; + }), + 'granted', + ], + [ + 'dumpsys fails', + fakeAdb((flat) => (flat === DUMPSYS ? { stderr: 'error', exitCode: 1 } : '0')), + 'unknown', + ], + [ + 'dumpsys output is unparseable', + fakeAdb((flat) => (flat === DUMPSYS ? 'Packages:' : '0')), + 'unknown', + ], + [ + 'the acting user cannot be resolved', + fakeAdb((flat) => + flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : dumpsys([{ id: 0 }]), + ), + 'unknown', + ], +] as const)( + 'setAndroidSetting permission deny reports %s', + async (_label, script, priorGrantState) => { + await withFakeAdb(script, async ({ device }) => { + const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + const warning = androidRevokedPermissionWarning( + 'com.example.app', + MICROPHONE, + priorGrantState, + ); + assert.deepEqual(result, { + permission: MICROPHONE, + priorGrantState, + ...(warning ? { warnings: [warning] } : {}), + }); + // not_granted is the only silent state; the other two hand over the same recovery. + assert.equal(warning === undefined, priorGrantState === 'not_granted'); + if (warning) assert.match(warning, /open com\.example\.app --relaunch/); + }); + }, +); + +test('the revoke warning states the platform rule and keeps the consequence conditional', () => { + // The read proves neither that the app was running nor, for `unknown`, what the state was. + assert.match( + androidRevokedPermissionWarning('com.example.app', MICROPHONE, 'granted')!, + /was granted before this revoke.*if com\.example\.app was running it is no longer/s, + ); + assert.match( + androidRevokedPermissionWarning('com.example.app', MICROPHONE, 'unknown')!, + /could not be read.*may no longer be running/s, + ); +}); + +// No resolvable user means no way to address one: the mutation keeps the platform default. +test('setAndroidSetting permission deny omits --user when the foreground user is unknown', async () => { + await withFakeAdb( + fakeAdb((flat) => + flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : undefined, + ), + async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { + permissionTarget: 'microphone', + }); + assert.deepEqual(calls, [ + ['shell', 'am', 'get-current-user'], + ['shell', 'pm', 'revoke', 'com.example.app', MICROPHONE], + ]); + }, + ); +}); + +// `photos` is the one target whose permission is discovered by probing the device, so its +// SDK-dependent candidate order and the flags that follow the resolved permission are pinned. +test.each([ + ['36', 'reset' as const, 'android.permission.READ_MEDIA_IMAGES'], + ['32', 'grant' as const, 'android.permission.READ_EXTERNAL_STORAGE'], +] as const)( + 'setAndroidSetting permission photos on SDK %s resolves %s to %s', + async (sdk, action, permission) => { + // `reset` maps to `pm revoke`; only `grant` keeps its verb. + const pmAction = action === 'grant' ? 'grant' : 'revoke'; + await withFakeAdb( + fakeAdb((flat) => { + if (flat === 'shell getprop ro.build.version.sdk') return sdk; + if (flat === CURRENT_USER) return '0'; + if (flat === DUMPSYS) return dumpsys([{ id: 0, runtime: [[permission, true]] }]); + if (flat.startsWith(`shell pm ${pmAction} --user 0 com.example.app ${permission}`)) + return ''; + return { stderr: `unexpected args: ${flat}`, exitCode: 1 }; + }), + async ({ calls, device }) => { + await setAndroidSetting(device, 'permission', action, 'com.example.app', { + permissionTarget: 'photos', + }); + const flat = calls.map((args) => args.join(' ')); + assert.ok(flat.includes('shell getprop ro.build.version.sdk'), flat.join('; ')); + assert.ok( + flat.includes(`shell pm ${pmAction} --user 0 com.example.app ${permission}`), + flat.join('; '), + ); + if (action === 'reset') { + for (const flag of ['user-set', 'user-fixed']) { + assert.ok( + flat.includes( + `shell pm clear-permission-flags --user 0 com.example.app ${permission} ${flag}`, + ), + flat.join('; '), + ); + } + } + }, + ); + }, +); + +test.each([ + [ + 'mode outside photos', + { permissionTarget: 'camera', permissionMode: 'limited' }, + /mode is only supported for photos/i, + ], + [ + 'an iOS-only target', + { permissionTarget: 'calendar' }, + /Unsupported permission target on Android/i, + ], +] as const)('setAndroidSetting permission rejects %s', async (_label, options, message) => { + await assertRejectsAppError( + () => setAndroidSetting(ANDROID_EMULATOR, 'permission', 'grant', 'com.example.app', options), + { code: 'INVALID_ARGS', message }, + ); +}); + +test('setAndroidSetting permission requires an app in session', async () => { + await assertRejectsAppError( + () => + setAndroidSetting(ANDROID_EMULATOR, 'permission', 'deny', undefined, { + permissionTarget: 'camera', + }), + { code: 'INVALID_ARGS', message: /requires an active app in session/ }, + ); +}); diff --git a/src/platforms/android/__tests__/settings.test.ts b/src/platforms/android/__tests__/settings.test.ts index c349c9665..4fa0f9f6e 100644 --- a/src/platforms/android/__tests__/settings.test.ts +++ b/src/platforms/android/__tests__/settings.test.ts @@ -1,6 +1,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; -import { androidRevokedPermissionWarning, setAndroidSetting } from '../settings.ts'; +import { setAndroidSetting } from '../settings.ts'; import { ANDROID_EMULATOR, assertRejectsAppError, @@ -124,496 +124,3 @@ test('setAndroidSetting fingerprint does not use adb emu command on physical dev }, ); }); - -test('setAndroidSetting permission deny notifications revokes runtime permission and appops', async () => { - await withFakeAdb( - () => undefined, - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'notifications', - }); - const flat = calls.map((args) => args.join(' ')); - assert.ok( - flat.includes('shell pm revoke com.example.app android.permission.POST_NOTIFICATIONS'), - flat.join('; '), - ); - assert.ok( - flat.includes('shell appops set com.example.app POST_NOTIFICATION deny'), - flat.join('; '), - ); - }, - ); -}); - -test('setAndroidSetting permission reset notifications clears permission flags for reprompt', async () => { - await withFakeAdb( - () => undefined, - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'notifications', - }); - const flat = calls.map((args) => args.join(' ')); - assert.ok( - flat.includes('shell pm revoke com.example.app android.permission.POST_NOTIFICATIONS'), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.POST_NOTIFICATIONS user-set', - ), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.POST_NOTIFICATIONS user-fixed', - ), - flat.join('; '), - ); - assert.ok( - flat.includes('shell appops set com.example.app POST_NOTIFICATION default'), - flat.join('; '), - ); - }, - ); -}); - -// #1796: Android kills the app when a permission it holds is revoked. The acting user's prior -// state is read before `pm revoke`; the response reports it as a three-state typed field, because -// a failed or unreadable dump is NOT evidence that the app was left alone. -const CURRENT_USER = 'shell am get-current-user'; -const DUMPSYS = 'shell dumpsys package com.example.app'; -const REVOKE_MICROPHONE = - 'shell pm revoke --user 0 com.example.app android.permission.RECORD_AUDIO'; - -/** A dump shaped like the real one: an install-permission section, then per-user blocks. */ -function dumpsys( - users: ReadonlyArray<{ id: number; runtime?: ReadonlyArray<[string, boolean]> }>, -): string { - return [ - 'Packages:', - ' Package [com.example.app] (abc):', - ' install permissions:', - // Install permissions are granted for the package, not per user, and `pm revoke` cannot - // touch them — a scan that reads `granted=true` anywhere reports these as runtime grants. - ' android.permission.INTERNET: granted=true', - ' android.permission.RECORD_AUDIO: granted=true', - ...users.flatMap(({ id, runtime }) => [ - ` User ${id}: ceDataInode=0 installed=true`, - ...(runtime - ? [ - ' runtime permissions:', - ...runtime.map( - ([permission, granted]) => - ` ${permission}: granted=${granted}, flags=[ USER_SET]`, - ), - ] - : []), - ]), - // A later section repeats `User :` without any runtime block. - 'Queries:', - ' queryable via interaction:', - ' User 0:', - ].join('\n'); -} - -function fakeAdb( - script: (flat: string) => string | undefined | { stderr: string; exitCode: number }, -) { - return (args: string[]) => script(args.join(' ')); -} - -test.each(['deny', 'reset'] as const)( - 'setAndroidSetting permission %s reports a granted prior state and warns', - async (action) => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '0'; - if (flat === DUMPSYS) { - return dumpsys([{ id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }]); - } - return undefined; - }), - async ({ calls, device }) => { - const result = await setAndroidSetting(device, 'permission', action, 'com.example.app', { - permissionTarget: 'microphone', - }); - const flat = calls.map((args) => args.join(' ')); - // The state is read BEFORE the revoke: after it, dumpsys would already say false. - assert.ok(flat.indexOf(DUMPSYS) < flat.indexOf(REVOKE_MICROPHONE), flat.join('; ')); - assert.deepEqual(result, { - permission: 'android.permission.RECORD_AUDIO', - priorGrantState: 'granted', - warnings: [ - androidRevokedPermissionWarning( - 'com.example.app', - 'android.permission.RECORD_AUDIO', - 'granted', - ), - ], - }); - assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); - // Conditional on purpose: the read proves neither that the app was running nor that a - // grant seen elsewhere belonged to the acting user (#1796 review). - assert.match(String(result?.warnings), /if com\.example\.app was running it is no longer/); - }, - ); - }, -); - -test('setAndroidSetting permission deny stays quiet when the acting user did not hold it', async () => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '0'; - if (flat === DUMPSYS) { - return dumpsys([{ id: 0, runtime: [['android.permission.CAMERA', false]] }]); - } - return undefined; - }), - async ({ device }) => { - const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'camera', - }); - assert.deepEqual(result, { - permission: 'android.permission.CAMERA', - priorGrantState: 'not_granted', - }); - }, - ); -}); - -// The three ways the prior state is genuinely unknown. Each must report `unknown` and still -// hand over the relaunch guidance: asserting `not_granted` here would claim the app survived. -test.each([ - ['dumpsys fails', (flat: string) => (flat === DUMPSYS ? { stderr: 'error', exitCode: 1 } : '0')], - [ - 'dumpsys output is unparseable', - (flat: string) => (flat === DUMPSYS ? 'Packages:\n ' : '0'), - ], - [ - 'the acting user cannot be resolved', - (flat: string) => - flat === CURRENT_USER - ? { stderr: 'cmd: not found', exitCode: 1 } - : dumpsys([{ id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }]), - ], -] as const)( - 'setAndroidSetting permission deny reports unknown (not not_granted) when %s', - async (_label, script) => { - await withFakeAdb( - fakeAdb((flat) => script(flat) as string | { stderr: string; exitCode: number }), - async ({ device }) => { - const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.equal(result?.priorGrantState, 'unknown'); - assert.match(String(result?.warnings), /could not be read/); - assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); - }, - ); - }, -); - -// A grant held by another profile is not this revoke's business: `pm revoke` acts on the -// acting user, so user 10's grant must not make user 0's revoke claim the app was killed. -test("setAndroidSetting permission deny reads only the acting user's block", async () => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '0'; - if (flat === DUMPSYS) { - return dumpsys([ - { id: 0, runtime: [['android.permission.RECORD_AUDIO', false]] }, - { id: 10, runtime: [['android.permission.RECORD_AUDIO', true]] }, - ]); - } - return undefined; - }), - async ({ device }) => { - const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.deepEqual(result, { - permission: 'android.permission.RECORD_AUDIO', - priorGrantState: 'not_granted', - }); - }, - ); -}); - -// The read scopes state to the foreground user, so the mutation has to name the same one: -// PackageManagerShellCommand defaults grant/revoke/permission-flag operations to -// UserHandle.USER_SYSTEM, so a bare `pm revoke` on a device whose foreground user is 10 edits -// user 0 and leaves the running app untouched. Proven on a Pixel 7 / API 36 emulator with the -// foreground user switched to 10: bare `pm revoke` flipped User 0 to granted=false while -// User 10 stayed granted=true. These pin the exact argv and order, because a response-shape -// assertion passes either way. -test.each([ - [ - 'deny microphone', - { permissionTarget: 'microphone' } as const, - 'deny' as const, - [ - ['shell', 'am', 'get-current-user'], - ['shell', 'dumpsys', 'package', 'com.example.app'], - [ - 'shell', - 'pm', - 'revoke', - '--user', - '10', - 'com.example.app', - 'android.permission.RECORD_AUDIO', - ], - ], - ], - [ - 'reset camera', - { permissionTarget: 'camera' } as const, - 'reset' as const, - [ - ['shell', 'am', 'get-current-user'], - ['shell', 'dumpsys', 'package', 'com.example.app'], - ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.CAMERA'], - // prettier-ignore - ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-set'], - // prettier-ignore - ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.CAMERA', 'user-fixed'], - ], - ], - [ - 'reset notifications', - { permissionTarget: 'notifications' } as const, - 'reset' as const, - [ - ['shell', 'am', 'get-current-user'], - ['shell', 'dumpsys', 'package', 'com.example.app'], - // prettier-ignore - ['shell', 'pm', 'revoke', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS'], - // prettier-ignore - ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-set'], - // prettier-ignore - ['shell', 'pm', 'clear-permission-flags', '--user', '10', 'com.example.app', 'android.permission.POST_NOTIFICATIONS', 'user-fixed'], - // prettier-ignore - ['shell', 'appops', 'set', '--user', '10', 'com.example.app', 'POST_NOTIFICATION', 'default'], - ], - ], -] as const)( - 'setAndroidSetting permission %s addresses the foreground user in every adb call', - async (_label, options, action, expected) => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '10'; - if (flat === DUMPSYS) { - return dumpsys([ - { id: 0, runtime: [['android.permission.RECORD_AUDIO', true]] }, - { id: 10, runtime: [['android.permission.RECORD_AUDIO', false]] }, - ]); - } - return undefined; - }), - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', action, 'com.example.app', options); - assert.deepEqual( - calls, - expected.map((args) => [...args]), - ); - }, - ); - }, -); - -test('setAndroidSetting permission grant addresses the foreground user too', async () => { - await withFakeAdb( - fakeAdb((flat) => (flat === CURRENT_USER ? '10' : undefined)), - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.deepEqual(calls, [ - ['shell', 'am', 'get-current-user'], - // prettier-ignore - ['shell', 'pm', 'grant', '--user', '10', 'com.example.app', 'android.permission.RECORD_AUDIO'], - ]); - }, - ); -}); - -// The read and the mutation must agree about WHICH user, not merely both name one: with the -// foreground user 10 holding the permission and user 0 not, the response must report granted. -test('setAndroidSetting permission deny reads the same user it revokes', async () => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '10'; - if (flat === DUMPSYS) { - return dumpsys([ - { id: 0, runtime: [['android.permission.RECORD_AUDIO', false]] }, - { id: 10, runtime: [['android.permission.RECORD_AUDIO', true]] }, - ]); - } - return undefined; - }), - async ({ device }) => { - const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.equal(result?.priorGrantState, 'granted'); - assert.match(String(result?.warnings), /open com\.example\.app --relaunch/); - }, - ); -}); - -// No resolvable user means no way to address one: the mutation keeps the platform default and -// the state is reported unknown rather than read from a user the revoke may not have touched. -test('setAndroidSetting permission deny omits --user when the foreground user is unknown', async () => { - await withFakeAdb( - fakeAdb((flat) => - flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : undefined, - ), - async ({ calls, device }) => { - const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.deepEqual(calls, [ - ['shell', 'am', 'get-current-user'], - ['shell', 'pm', 'revoke', 'com.example.app', 'android.permission.RECORD_AUDIO'], - ]); - assert.equal(result?.priorGrantState, 'unknown'); - }, - ); -}); - -test('setAndroidSetting permission grant does not read grant state', async () => { - await withFakeAdb( - fakeAdb((flat) => (flat === CURRENT_USER ? '0' : undefined)), - async ({ calls, device }) => { - const result = await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { - permissionTarget: 'camera', - }); - assert.equal(result, undefined); - // The user is resolved for the mutation, but no state is read: grant cannot kill the app. - assert.deepEqual(calls, [ - ['shell', 'am', 'get-current-user'], - ['shell', 'pm', 'grant', '--user', '0', 'com.example.app', 'android.permission.CAMERA'], - ]); - }, - ); -}); - -test('setAndroidSetting permission reset notifications reports the POST_NOTIFICATIONS state', async () => { - await withFakeAdb( - fakeAdb((flat) => { - if (flat === CURRENT_USER) return '0'; - if (flat === DUMPSYS) { - return dumpsys([{ id: 0, runtime: [['android.permission.POST_NOTIFICATIONS', true]] }]); - } - return undefined; - }), - async ({ device }) => { - const result = await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'notifications', - }); - assert.equal(result?.priorGrantState, 'granted'); - assert.equal(Array.isArray(result?.warnings), true); - }, - ); -}); - -test('setAndroidSetting permission reset camera clears permission flags for reprompt', async () => { - await withFakeAdb( - () => undefined, - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'camera', - }); - const flat = calls.map((args) => args.join(' ')); - assert.ok( - flat.includes('shell pm revoke com.example.app android.permission.CAMERA'), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.CAMERA user-set', - ), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.CAMERA user-fixed', - ), - flat.join('; '), - ); - }, - ); -}); - -test('setAndroidSetting permission reset photos clears flags for the resolved permission', async () => { - await withFakeAdb( - (args) => (args.join(' ') === 'shell getprop ro.build.version.sdk' ? '36' : undefined), - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', { - permissionTarget: 'photos', - }); - const flat = calls.map((args) => args.join(' ')); - assert.ok( - flat.includes('shell pm revoke com.example.app android.permission.READ_MEDIA_IMAGES'), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.READ_MEDIA_IMAGES user-set', - ), - flat.join('; '), - ); - assert.ok( - flat.includes( - 'shell pm clear-permission-flags com.example.app android.permission.READ_MEDIA_IMAGES user-fixed', - ), - flat.join('; '), - ); - }, - ); -}); - -test('setAndroidSetting permission rejects mode argument', async () => { - await assertRejectsAppError( - () => - setAndroidSetting(ANDROID_EMULATOR, 'permission', 'grant', 'com.example.app', { - permissionTarget: 'camera', - permissionMode: 'limited', - }), - { code: 'INVALID_ARGS', message: /mode is only supported for photos/i }, - ); -}); - -test('setAndroidSetting permission rejects iOS-only targets with Android-specific guidance', async () => { - await assertRejectsAppError( - () => - setAndroidSetting(ANDROID_EMULATOR, 'permission', 'grant', 'com.example.app', { - permissionTarget: 'calendar', - }), - { code: 'INVALID_ARGS', message: /Unsupported permission target on Android/i }, - ); -}); - -test('setAndroidSetting permission grant photos falls back to legacy permission on older SDK', async () => { - await withFakeAdb( - (args) => { - const flat = args.join(' '); - if (flat === 'shell getprop ro.build.version.sdk') return '32'; - if (flat === 'shell pm grant com.example.app android.permission.READ_EXTERNAL_STORAGE') { - return ''; - } - return { stderr: `unexpected args: ${flat}`, exitCode: 1 }; - }, - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', { - permissionTarget: 'photos', - }); - const flat = calls.map((args) => args.join(' ')); - assert.ok(flat.includes('shell getprop ro.build.version.sdk'), flat.join('; ')); - assert.ok( - flat.includes('shell pm grant com.example.app android.permission.READ_EXTERNAL_STORAGE'), - flat.join('; '), - ); - }, - ); -}); diff --git a/src/platforms/android/permission-grant-state.ts b/src/platforms/android/permission-grant-state.ts index d63b75b21..7406b8f05 100644 --- a/src/platforms/android/permission-grant-state.ts +++ b/src/platforms/android/permission-grant-state.ts @@ -13,23 +13,15 @@ import { runAndroidAdb } from './adb.ts'; export type AndroidPriorGrantState = 'granted' | 'not_granted' | 'unknown'; /** - * The grants a revoke's prior state is resolved against. Held opaque so a caller reads the - * state once, before the revoke, and can still answer for a permission it only learns - * afterwards (`photos` resolves its permission by probing the device). + * What the acting user actually holds. `unknown` is never a value here — absence is what carries + * it, so a caller reads the map before the revoke and can still answer for a permission it only + * learns afterwards (`photos` resolves its permission by probing the device). */ -export type AndroidRuntimePermissionGrants = ReadonlyMap; - -/** The state of one permission within a grants read; `unknown` when either is absent. */ -export function androidPriorGrantState( - grants: AndroidRuntimePermissionGrants | undefined, - permission: string, -): AndroidPriorGrantState { - return grants?.get(permission) ?? 'unknown'; -} +export type AndroidRuntimePermissionGrants = ReadonlyMap; /** - * `userId`'s runtime permissions, or `undefined` when the state could not be read — adb failed - * or the dump carried no runtime-permission block for that user. + * `userId`'s runtime permissions, or `undefined` when the state could not be read — no acting + * user, adb failed, or the dump carried no runtime-permission block for that user. * * The caller passes the user its mutation will target, so the two halves cannot disagree. * `dumpsys package` prints an `install permissions:` section and one block per user, all @@ -39,8 +31,9 @@ export function androidPriorGrantState( export async function readAndroidRuntimePermissionGrants( device: DeviceInfo, appPackage: string, - userId: number, + userId: number | undefined, ): Promise { + if (userId === undefined) return undefined; const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { allowFailure: true, }); @@ -62,12 +55,11 @@ export async function readAndroidCurrentUserId(device: DeviceInfo): Promise= 0 ? parsed : undefined; } -const TOP_LEVEL_SECTION = /^\S.*:\s*$/; -const PACKAGES_SECTION = 'Packages:'; const USER_BLOCK = /^\s*User (\d+):/; const RUNTIME_PERMISSIONS_BLOCK = /^\s*runtime permissions:\s*$/; const GRANT_LINE = /^\s*([\w.]+): granted=(true|false)\b/; @@ -79,24 +71,30 @@ type DumpLine = { text: string; indent: number }; * Runtime permission grants for `userId` only, or `undefined` when that user has no * runtime-permission block in the dump. * - * `dumpsys package` is indentation-structured: top-level `Packages:` holds the package's own - * blocks, `User :` opens a per-user block, and `runtime permissions:` opens the grant list - * inside it. The read walks that nesting instead of matching `granted=` anywhere, which is what - * keeps the `install permissions:` section, other users' blocks, and the later top-level - * sections (`Queries:`, `Shared users:`, `Dexopt state:` — which repeat `User :` and can - * repeat grant lines) out of the answer. + * `dumpsys package` is indentation-structured, so the read walks three nested blocks — + * `Packages:` → `User :` → `runtime permissions:` — instead of matching `granted=` + * anywhere. That nesting is what keeps the `install permissions:` section, other users' + * blocks, and the later top-level sections (`Queries:`, `Shared users:`, `Dexopt state:`, + * which repeat `User :` and can repeat grant lines) out of the answer. */ export function parseAndroidRuntimePermissionGrants( dumpsysOutput: string, userId: number, ): AndroidRuntimePermissionGrants | undefined { - const packages = topLevelSection(readDumpLines(dumpsysOutput), PACKAGES_SECTION); + const lines = dumpsysOutput + .split('\n') + .filter((text) => text.trim().length > 0) + .map((text) => ({ text, indent: text.length - text.trimStart().length })); + const packages = nestedBlock( + lines, + (line) => line.indent === 0 && line.text.trim() === 'Packages:', + ); const user = nestedBlock(packages, (line) => USER_BLOCK.exec(line.text)?.[1] === String(userId)); const runtime = nestedBlock(user, (line) => RUNTIME_PERMISSIONS_BLOCK.test(line.text)); // Absent block: the device never reported this user's grants. An empty one is still an // answer — the app holds no runtime permissions for this user. if (!runtime) return undefined; - const grants = new Map(); + const grants = new Map(); for (const { text } of runtime) { const grant = GRANT_LINE.exec(text); if (grant) grants.set(grant[1]!, grant[2] === 'true' ? 'granted' : 'not_granted'); @@ -104,23 +102,11 @@ export function parseAndroidRuntimePermissionGrants( return grants; } -function readDumpLines(dumpsysOutput: string): DumpLine[] { - return dumpsysOutput - .split('\n') - .filter((text) => text.trim().length > 0) - .map((text) => ({ text, indent: text.length - text.trimStart().length })); -} - -/** The lines under a top-level `` header, up to the next top-level header. */ -function topLevelSection(lines: readonly DumpLine[], name: string): DumpLine[] { - const start = lines.findIndex((line) => line.indent === 0 && line.text.trim() === name); - if (start < 0) return []; - const rest = lines.slice(start + 1); - const end = rest.findIndex((line) => line.indent === 0 && TOP_LEVEL_SECTION.test(line.text)); - return end < 0 ? rest : rest.slice(0, end); -} - -/** The lines nested under the first line `isHeader` accepts, or `undefined` if there is none. */ +/** + * The lines nested under the first line `isHeader` accepts, or `undefined` if there is none. + * A block ends at the first line indented no deeper than its header, which is also what ends + * the top-level `Packages:` section at the next top-level heading. + */ function nestedBlock( lines: readonly DumpLine[] | undefined, isHeader: (line: DumpLine) => boolean, diff --git a/src/platforms/android/settings-permission.ts b/src/platforms/android/settings-permission.ts new file mode 100644 index 000000000..64c7f94a2 --- /dev/null +++ b/src/platforms/android/settings-permission.ts @@ -0,0 +1,238 @@ +import { AppError } from '@agent-device/kernel/errors'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { parsePermissionAction, parsePermissionTarget } from '@agent-device/contracts/settings'; +import type { SettingOptions } from '@agent-device/contracts/settings'; +import { runAndroidAdb } from './adb.ts'; +import { + readAndroidCurrentUserId, + readAndroidRuntimePermissionGrants, + type AndroidPriorGrantState, +} from './permission-grant-state.ts'; + +/** + * Android kills the app's process whenever a runtime permission it currently holds is + * revoked (`pm revoke` after a grant, foreground or background), so a `deny`/`reset` that + * follows a grant leaves the session pointing at a dead app and the next selector fails + * against the launcher (#1796). Revoking a permission the app does not hold is harmless. + * + * Process death itself is NOT observed (that would be option (b) in the issue) and the prior + * state cannot prove the app was running, so the consequence stays conditional. When the state + * could not be read, the same guidance is given without claiming what the state was: silence + * there would assert "your app is untouched" on no evidence. + */ +export function androidRevokedPermissionWarning( + appPackage: string, + permission: string, + priorGrantState: AndroidPriorGrantState, +): string | undefined { + if (priorGrantState === 'not_granted') return undefined; + const preamble = + priorGrantState === 'granted' + ? `${permission} was granted before this revoke, and Android kills an app when a granted permission is revoked: if ${appPackage} was running it is no longer.` + : `Whether ${permission} was granted before this revoke could not be read (adb did not report the acting user's runtime permission state), and Android kills an app when a granted permission is revoked: ${appPackage} may no longer be running.`; + return `${preamble} Relaunch it with open ${appPackage} --relaunch before the next interaction.`; +} + +type AndroidPermissionTarget = ReturnType; + +/** + * `--user ` for every permission mutation, resolved once so the state read and the mutation + * cannot address different users. Empty only when the foreground user could not be resolved, in + * which case the platform default (`UserHandle.USER_SYSTEM`) applies and the prior state is + * reported as unknown rather than guessed. + */ +type AndroidUserArgs = readonly string[]; + +export async function setAndroidPermission( + device: DeviceInfo, + appPackage: string, + state: string, + options: SettingOptions | undefined, +): Promise | void> { + const action = parsePermissionAction(state); + const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); + const userId = await readAndroidCurrentUserId(device); + const userArgs: AndroidUserArgs = userId === undefined ? [] : ['--user', String(userId)]; + if (action === 'grant') { + await grantAndroidPermission(device, appPackage, target, userArgs); + return; + } + // Read before the revoke — afterwards every permission reads as not granted — but resolved + // after it, because `photos` only learns which permission it revoked by probing the device. + const grants = await readAndroidRuntimePermissionGrants(device, appPackage, userId); + const permission = await revokeAndroidPermission(device, appPackage, action, target, userArgs); + const priorGrantState: AndroidPriorGrantState = grants?.get(permission) ?? 'unknown'; + const warning = androidRevokedPermissionWarning(appPackage, permission, priorGrantState); + return { + permission, + priorGrantState, + ...(warning ? { warnings: [warning] } : {}), + }; +} + +async function grantAndroidPermission( + device: DeviceInfo, + appPackage: string, + target: AndroidPermissionTarget, + userArgs: AndroidUserArgs, +): Promise { + if (target.kind === 'notifications') { + await setAndroidNotificationPermission(device, appPackage, 'grant', target, userArgs); + } else if (target.type === 'photos') { + await setAndroidPhotoPermission(device, appPackage, 'grant', userArgs); + } else { + await runAndroidAdb(device, ['shell', 'pm', 'grant', ...userArgs, appPackage, target.value]); + } +} + +/** Revokes (and for `reset`, clears the flags of) the target; returns the permission revoked. */ +async function revokeAndroidPermission( + device: DeviceInfo, + appPackage: string, + action: 'deny' | 'reset', + target: AndroidPermissionTarget, + userArgs: AndroidUserArgs, +): Promise { + if (target.kind === 'notifications') { + await setAndroidNotificationPermission(device, appPackage, action, target, userArgs); + return target.permission; + } + let permission: string; + if (target.type === 'photos') { + permission = await setAndroidPhotoPermission(device, appPackage, 'revoke', userArgs); + } else { + permission = target.value; + await runAndroidAdb(device, ['shell', 'pm', 'revoke', ...userArgs, appPackage, permission]); + } + if (action === 'reset') { + await clearAndroidPermissionFlags(device, appPackage, permission, userArgs); + } + return permission; +} + +function parseAndroidPermissionTarget( + permissionTarget: string | undefined, + permissionMode: string | undefined, +): + | { kind: 'pm'; value: string; type: 'camera' | 'microphone' | 'photos' | 'contacts' } + | { kind: 'notifications'; appOps: string; permission: string } { + const normalized = parsePermissionTarget(permissionTarget); + if (permissionMode?.trim()) { + throw new AppError( + 'INVALID_ARGS', + `Permission mode is only supported for photos. Received: ${permissionMode}.`, + ); + } + if (normalized === 'camera') + return { kind: 'pm', value: 'android.permission.CAMERA', type: 'camera' }; + if (normalized === 'microphone') { + return { kind: 'pm', value: 'android.permission.RECORD_AUDIO', type: 'microphone' }; + } + if (normalized === 'photos') { + return { kind: 'pm', value: 'android.permission.READ_MEDIA_IMAGES', type: 'photos' }; + } + if (normalized === 'contacts') { + return { kind: 'pm', value: 'android.permission.READ_CONTACTS', type: 'contacts' }; + } + if (normalized === 'notifications') { + return { + kind: 'notifications', + appOps: 'POST_NOTIFICATION', + permission: 'android.permission.POST_NOTIFICATIONS', + }; + } + throw new AppError( + 'INVALID_ARGS', + `Unsupported permission target on Android: ${permissionTarget}. Use camera|microphone|photos|contacts|notifications.`, + ); +} + +async function setAndroidPhotoPermission( + device: DeviceInfo, + appPackage: string, + pmAction: 'grant' | 'revoke', + userArgs: AndroidUserArgs, +): Promise { + const sdkInt = await getAndroidSdkInt(device); + const candidates = + sdkInt !== null && sdkInt >= 33 + ? ['android.permission.READ_MEDIA_IMAGES', 'android.permission.READ_EXTERNAL_STORAGE'] + : ['android.permission.READ_EXTERNAL_STORAGE', 'android.permission.READ_MEDIA_IMAGES']; + + const failures: Array<{ permission: string; stderr: string; exitCode: number }> = []; + for (const permission of candidates) { + const result = await runAndroidAdb( + device, + ['shell', 'pm', pmAction, ...userArgs, appPackage, permission], + { allowFailure: true }, + ); + if (result.exitCode === 0) return permission; + failures.push({ permission, stderr: result.stderr, exitCode: result.exitCode }); + } + + throw new AppError('COMMAND_FAILED', `Failed to ${pmAction} Android photos permission`, { + appPackage, + sdkInt, + attempts: failures, + }); +} + +async function setAndroidNotificationPermission( + device: DeviceInfo, + appPackage: string, + action: 'grant' | 'deny' | 'reset', + target: { appOps: string; permission: string }, + userArgs: AndroidUserArgs, +): Promise { + const appOpsMode = action === 'grant' ? 'allow' : action === 'deny' ? 'deny' : 'default'; + if (action === 'grant') { + await runAndroidAdb( + device, + ['shell', 'pm', 'grant', ...userArgs, appPackage, target.permission], + { allowFailure: true }, + ); + } else { + await runAndroidAdb( + device, + ['shell', 'pm', 'revoke', ...userArgs, appPackage, target.permission], + { allowFailure: true }, + ); + if (action === 'reset') { + await clearAndroidPermissionFlags(device, appPackage, target.permission, userArgs); + } + } + await runAndroidAdb(device, [ + 'shell', + 'appops', + 'set', + ...userArgs, + appPackage, + target.appOps, + appOpsMode, + ]); +} + +async function clearAndroidPermissionFlags( + device: DeviceInfo, + appPackage: string, + permission: string, + userArgs: AndroidUserArgs, +): Promise { + for (const flag of ['user-set', 'user-fixed']) { + await runAndroidAdb( + device, + ['shell', 'pm', 'clear-permission-flags', ...userArgs, appPackage, permission, flag], + { allowFailure: true }, + ); + } +} + +async function getAndroidSdkInt(device: DeviceInfo): Promise { + const result = await runAndroidAdb(device, ['shell', 'getprop', 'ro.build.version.sdk'], { + allowFailure: true, + }); + if (result.exitCode !== 0) return null; + const value = Number.parseInt(result.stdout.trim(), 10); + if (!Number.isFinite(value) || value <= 0) return null; + return value; +} diff --git a/src/platforms/android/settings.ts b/src/platforms/android/settings.ts index d04ac4657..b4b7825cc 100644 --- a/src/platforms/android/settings.ts +++ b/src/platforms/android/settings.ts @@ -5,22 +5,13 @@ import { summarizeCommandAttemptFailures, type CommandAttemptFailure, } from '../command-attempts.ts'; -import { - parsePermissionAction, - parsePermissionTarget, - type SettingOptions, -} from '@agent-device/contracts/settings'; +import type { SettingOptions } from '@agent-device/contracts/settings'; import { parseAppearanceAction } from '../appearance.ts'; import { parseSettingState } from '../setting-state.ts'; import { runAndroidAdb } from './adb.ts'; import { androidAdbResultError } from './adb-executor.ts'; import { resolveAndroidApp } from './app-deployment-resolution.ts'; -import { - androidPriorGrantState, - readAndroidCurrentUserId, - readAndroidRuntimePermissionGrants, - type AndroidPriorGrantState, -} from './permission-grant-state.ts'; +import { setAndroidPermission } from './settings-permission.ts'; const ANDROID_ANIMATION_SCALE_SETTINGS = [ 'window_animation_scale', @@ -155,110 +146,6 @@ export async function setAndroidSetting( } } -/** - * Android kills the app's process whenever a runtime permission it currently holds is - * revoked (`pm revoke` after a grant, foreground or background), so a `deny`/`reset` that - * follows a grant leaves the session pointing at a dead app and the next selector fails - * against the launcher (#1796). Revoking a permission the app does not hold is harmless. - * - * Process death itself is NOT observed (that would be option (b) in the issue) and the prior - * state cannot prove the app was running, so the consequence stays conditional. When the state - * could not be read, the same guidance is given without claiming what the state was: silence - * there would assert "your app is untouched" on no evidence. - */ -export function androidRevokedPermissionWarning( - appPackage: string, - permission: string, - priorGrantState: AndroidPriorGrantState, -): string | undefined { - if (priorGrantState === 'not_granted') return undefined; - const preamble = - priorGrantState === 'granted' - ? `${permission} was granted before this revoke, and Android kills an app when a granted permission is revoked: if ${appPackage} was running it is no longer.` - : `Whether ${permission} was granted before this revoke could not be read (adb did not report the acting user's runtime permission state), and Android kills an app when a granted permission is revoked: ${appPackage} may no longer be running.`; - return `${preamble} Relaunch it with open ${appPackage} --relaunch before the next interaction.`; -} - -type AndroidPermissionTarget = ReturnType; - -/** - * `--user ` for every permission mutation, resolved once so the state read and the mutation - * cannot address different users. Empty only when the foreground user could not be resolved, in - * which case the platform default (`UserHandle.USER_SYSTEM`) applies and the prior state is - * reported as unknown rather than guessed. - */ -type AndroidUserArgs = readonly string[]; - -async function setAndroidPermission( - device: DeviceInfo, - appPackage: string, - state: string, - options: SettingOptions | undefined, -): Promise | void> { - const action = parsePermissionAction(state); - const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); - const userId = await readAndroidCurrentUserId(device); - const userArgs: AndroidUserArgs = userId === undefined ? [] : ['--user', String(userId)]; - if (action === 'grant') { - await grantAndroidPermission(device, appPackage, target, userArgs); - return; - } - // Read before the revoke — afterwards every permission reads as not granted — but resolved - // after it, because `photos` only learns which permission it revoked by probing the device. - const grants = - userId === undefined - ? undefined - : await readAndroidRuntimePermissionGrants(device, appPackage, userId); - const permission = await revokeAndroidPermission(device, appPackage, action, target, userArgs); - const priorGrantState = androidPriorGrantState(grants, permission); - const warning = androidRevokedPermissionWarning(appPackage, permission, priorGrantState); - return { - permission, - priorGrantState, - ...(warning ? { warnings: [warning] } : {}), - }; -} - -async function grantAndroidPermission( - device: DeviceInfo, - appPackage: string, - target: AndroidPermissionTarget, - userArgs: AndroidUserArgs, -): Promise { - if (target.kind === 'notifications') { - await setAndroidNotificationPermission(device, appPackage, 'grant', target, userArgs); - } else if (target.type === 'photos') { - await setAndroidPhotoPermission(device, appPackage, 'grant', userArgs); - } else { - await runAndroidAdb(device, ['shell', 'pm', 'grant', ...userArgs, appPackage, target.value]); - } -} - -/** Revokes (and for `reset`, clears the flags of) the target; returns the permission revoked. */ -async function revokeAndroidPermission( - device: DeviceInfo, - appPackage: string, - action: 'deny' | 'reset', - target: AndroidPermissionTarget, - userArgs: AndroidUserArgs, -): Promise { - if (target.kind === 'notifications') { - await setAndroidNotificationPermission(device, appPackage, action, target, userArgs); - return target.permission; - } - let permission: string; - if (target.type === 'photos') { - permission = await setAndroidPhotoPermission(device, appPackage, 'revoke', userArgs); - } else { - permission = target.value; - await runAndroidAdb(device, ['shell', 'pm', 'revoke', ...userArgs, appPackage, permission]); - } - if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, permission, userArgs); - } - return permission; -} - type AndroidFingerprintAction = 'match' | 'nonmatch'; function parseAndroidFingerprintAction(state: string): AndroidFingerprintAction { @@ -376,130 +263,3 @@ function parseAndroidAppearance(stdout: string, stderr: string): 'light' | 'dark if (value === 'auto') return 'auto'; return null; } - -function parseAndroidPermissionTarget( - permissionTarget: string | undefined, - permissionMode: string | undefined, -): - | { kind: 'pm'; value: string; type: 'camera' | 'microphone' | 'photos' | 'contacts' } - | { kind: 'notifications'; appOps: string; permission: string } { - const normalized = parsePermissionTarget(permissionTarget); - if (permissionMode?.trim()) { - throw new AppError( - 'INVALID_ARGS', - `Permission mode is only supported for photos. Received: ${permissionMode}.`, - ); - } - if (normalized === 'camera') - return { kind: 'pm', value: 'android.permission.CAMERA', type: 'camera' }; - if (normalized === 'microphone') { - return { kind: 'pm', value: 'android.permission.RECORD_AUDIO', type: 'microphone' }; - } - if (normalized === 'photos') { - return { kind: 'pm', value: 'android.permission.READ_MEDIA_IMAGES', type: 'photos' }; - } - if (normalized === 'contacts') { - return { kind: 'pm', value: 'android.permission.READ_CONTACTS', type: 'contacts' }; - } - if (normalized === 'notifications') { - return { - kind: 'notifications', - appOps: 'POST_NOTIFICATION', - permission: 'android.permission.POST_NOTIFICATIONS', - }; - } - throw new AppError( - 'INVALID_ARGS', - `Unsupported permission target on Android: ${permissionTarget}. Use camera|microphone|photos|contacts|notifications.`, - ); -} - -async function setAndroidPhotoPermission( - device: DeviceInfo, - appPackage: string, - pmAction: 'grant' | 'revoke', - userArgs: AndroidUserArgs, -): Promise { - const sdkInt = await getAndroidSdkInt(device); - const candidates = - sdkInt !== null && sdkInt >= 33 - ? ['android.permission.READ_MEDIA_IMAGES', 'android.permission.READ_EXTERNAL_STORAGE'] - : ['android.permission.READ_EXTERNAL_STORAGE', 'android.permission.READ_MEDIA_IMAGES']; - - const failures: Array<{ permission: string; stderr: string; exitCode: number }> = []; - for (const permission of candidates) { - const result = await runAndroidAdb( - device, - ['shell', 'pm', pmAction, ...userArgs, appPackage, permission], - { allowFailure: true }, - ); - if (result.exitCode === 0) return permission; - failures.push({ permission, stderr: result.stderr, exitCode: result.exitCode }); - } - - throw new AppError('COMMAND_FAILED', `Failed to ${pmAction} Android photos permission`, { - appPackage, - sdkInt, - attempts: failures, - }); -} - -async function setAndroidNotificationPermission( - device: DeviceInfo, - appPackage: string, - action: 'grant' | 'deny' | 'reset', - target: { appOps: string; permission: string }, - userArgs: AndroidUserArgs, -): Promise { - const appOpsMode = action === 'grant' ? 'allow' : action === 'deny' ? 'deny' : 'default'; - if (action === 'grant') { - await runAndroidAdb( - device, - ['shell', 'pm', 'grant', ...userArgs, appPackage, target.permission], - { allowFailure: true }, - ); - } else { - await runAndroidAdb( - device, - ['shell', 'pm', 'revoke', ...userArgs, appPackage, target.permission], - { allowFailure: true }, - ); - if (action === 'reset') { - await clearAndroidPermissionFlags(device, appPackage, target.permission, userArgs); - } - } - await runAndroidAdb(device, [ - 'shell', - 'appops', - 'set', - ...userArgs, - appPackage, - target.appOps, - appOpsMode, - ]); -} - -async function clearAndroidPermissionFlags( - device: DeviceInfo, - appPackage: string, - permission: string, - userArgs: AndroidUserArgs, -): Promise { - for (const flag of ['user-set', 'user-fixed']) { - await runAndroidAdb( - device, - ['shell', 'pm', 'clear-permission-flags', ...userArgs, appPackage, permission, flag], - { allowFailure: true }, - ); - } -} - -async function getAndroidSdkInt(device: DeviceInfo): Promise { - const result = await runAndroidAdb(device, ['shell', 'getprop', 'ro.build.version.sdk'], { - allowFailure: true, - }); - if (result.exitCode !== 0) return null; - const value = Number.parseInt(result.stdout.trim(), 10); - if (!Number.isFinite(value) || value <= 0) return null; - return value; -} From 0f5d1cc347634368366b608eabc637a9904b511b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 16:59:09 +0200 Subject: [PATCH 8/8] fix(android): refuse permission mutations that cannot name their user The fallback issued bare pm/appops commands when am get-current-user did not answer, which is the #1796 defect itself: those default to UserHandle.USER_SYSTEM, so a session running as user 10 had user 0 edited while the response reported only priorGrantState: unknown. It was also a fallback added without approval, and the docs' claim that every mutation names its user was false on that path. Resolving the acting user is now a prerequisite: setAndroidSetting permission fails with COMMAND_FAILED and a recovery hint, issuing no pm, appops or clear-permission-flags call at all. The test that locked the fallback in is replaced by one asserting the empty mutation call list for grant, deny and reset. --- src/commands/capture/settings.ts | 2 +- .../__tests__/settings-permission.test.ts | 52 ++++++++++--------- .../android/permission-grant-state.ts | 7 ++- src/platforms/android/settings-permission.ts | 31 +++++++++-- website/docs/docs/commands.md | 2 +- 5 files changed, 59 insertions(+), 35 deletions(-) diff --git a/src/commands/capture/settings.ts b/src/commands/capture/settings.ts index 98f1255fa..6d7b077b1 100644 --- a/src/commands/capture/settings.ts +++ b/src/commands/capture/settings.ts @@ -60,7 +60,7 @@ export const settingsCommandFacet = defineCommandFacet({ text: { summary: 'Change OS settings and app permissions', cliDetail: - 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills a running app; the response reports priorGrantState (granted|not_granted|unknown) and warns for granted and unknown, with open --relaunch to restore it.', + 'macOS supports only settings appearance and settings permission ; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app. On Android, deny|reset of a permission the app currently holds kills a running app; the response reports priorGrantState (granted|not_granted|unknown) and warns for granted and unknown, with open --relaunch to restore it. Permission changes require a resolvable foreground user and fail without mutating if adb cannot report one.', }, metadata: settingsCommandMetadata, definition: settingsCommandDefinition, diff --git a/src/platforms/android/__tests__/settings-permission.test.ts b/src/platforms/android/__tests__/settings-permission.test.ts index a48f49535..dda4ba199 100644 --- a/src/platforms/android/__tests__/settings-permission.test.ts +++ b/src/platforms/android/__tests__/settings-permission.test.ts @@ -187,13 +187,6 @@ test.each([ fakeAdb((flat) => (flat === DUMPSYS ? 'Packages:' : '0')), 'unknown', ], - [ - 'the acting user cannot be resolved', - fakeAdb((flat) => - flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : dumpsys([{ id: 0 }]), - ), - 'unknown', - ], ] as const)( 'setAndroidSetting permission deny reports %s', async (_label, script, priorGrantState) => { @@ -230,23 +223,34 @@ test('the revoke warning states the platform rule and keeps the consequence cond ); }); -// No resolvable user means no way to address one: the mutation keeps the platform default. -test('setAndroidSetting permission deny omits --user when the foreground user is unknown', async () => { - await withFakeAdb( - fakeAdb((flat) => - flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : undefined, - ), - async ({ calls, device }) => { - await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', { - permissionTarget: 'microphone', - }); - assert.deepEqual(calls, [ - ['shell', 'am', 'get-current-user'], - ['shell', 'pm', 'revoke', 'com.example.app', MICROPHONE], - ]); - }, - ); -}); +// A mutation that cannot name its user is refused, not issued unscoped: `pm` would apply it to +// user 0 and leave a session running as another user untouched, which is the whole defect. +test.each(['grant', 'deny', 'reset'] as const)( + 'setAndroidSetting permission %s refuses to mutate when the acting user cannot be resolved', + async (action) => { + await withFakeAdb( + fakeAdb((flat) => + flat === CURRENT_USER ? { stderr: 'cmd: not found', exitCode: 1 } : undefined, + ), + async ({ calls, device }) => { + await assertRejectsAppError( + () => + setAndroidSetting(device, 'permission', action, 'com.example.app', { + permissionTarget: 'microphone', + }), + { + code: 'COMMAND_FAILED', + message: /Could not determine which Android user/, + hint: /am get-current-user/, + }, + ); + // The load-bearing assertion: the resolution attempt is the ONLY adb call. No pm, no + // appops, no clear-permission-flags — nothing that could edit user 0's state. + assert.deepEqual(calls, [['shell', 'am', 'get-current-user']]); + }, + ); + }, +); // `photos` is the one target whose permission is discovered by probing the device, so its // SDK-dependent candidate order and the flags that follow the resolved permission are pinned. diff --git a/src/platforms/android/permission-grant-state.ts b/src/platforms/android/permission-grant-state.ts index 7406b8f05..5e0bfd18c 100644 --- a/src/platforms/android/permission-grant-state.ts +++ b/src/platforms/android/permission-grant-state.ts @@ -20,8 +20,8 @@ export type AndroidPriorGrantState = 'granted' | 'not_granted' | 'unknown'; export type AndroidRuntimePermissionGrants = ReadonlyMap; /** - * `userId`'s runtime permissions, or `undefined` when the state could not be read — no acting - * user, adb failed, or the dump carried no runtime-permission block for that user. + * `userId`'s runtime permissions, or `undefined` when the state could not be read — adb failed, + * or the dump carried no runtime-permission block for that user. * * The caller passes the user its mutation will target, so the two halves cannot disagree. * `dumpsys package` prints an `install permissions:` section and one block per user, all @@ -31,9 +31,8 @@ export type AndroidRuntimePermissionGrants = ReadonlyMap { - if (userId === undefined) return undefined; const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], { allowFailure: true, }); diff --git a/src/platforms/android/settings-permission.ts b/src/platforms/android/settings-permission.ts index 64c7f94a2..3c174d167 100644 --- a/src/platforms/android/settings-permission.ts +++ b/src/platforms/android/settings-permission.ts @@ -37,12 +37,33 @@ type AndroidPermissionTarget = ReturnType; /** * `--user ` for every permission mutation, resolved once so the state read and the mutation - * cannot address different users. Empty only when the foreground user could not be resolved, in - * which case the platform default (`UserHandle.USER_SYSTEM`) applies and the prior state is - * reported as unknown rather than guessed. + * cannot address different users. Never empty: a permission mutation that cannot name its user + * is refused rather than issued (see `requireAndroidPermissionUser`). */ type AndroidUserArgs = readonly string[]; +/** + * The user a permission mutation will act on, or a refusal. + * + * `pm` and `appops` default to `UserHandle.USER_SYSTEM`, so an unscoped mutation on a device + * whose foreground user is nonzero edits user 0 and leaves the running app untouched — the + * defect #1796 is about. Issuing the bare command as a fallback would reintroduce it on exactly + * the path where we already know we are guessing, so the command refuses instead: no permission + * state is changed when we cannot name whose state it is. + */ +async function requireAndroidPermissionUser(device: DeviceInfo): Promise { + const userId = await readAndroidCurrentUserId(device); + if (userId !== undefined) return userId; + throw new AppError( + 'COMMAND_FAILED', + 'Could not determine which Android user the session runs as, so no permission was changed.', + { + deviceId: device.id, + hint: `Check adb -s ${device.id} shell am get-current-user — if the device is still booting, retry once it reports a user. agent-device refuses to change permissions it cannot scope, because pm would silently apply them to user 0.`, + }, + ); +} + export async function setAndroidPermission( device: DeviceInfo, appPackage: string, @@ -51,8 +72,8 @@ export async function setAndroidPermission( ): Promise | void> { const action = parsePermissionAction(state); const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode); - const userId = await readAndroidCurrentUserId(device); - const userArgs: AndroidUserArgs = userId === undefined ? [] : ['--user', String(userId)]; + const userId = await requireAndroidPermissionUser(device); + const userArgs: AndroidUserArgs = ['--user', String(userId)]; if (action === 'grant') { await grantAndroidPermission(device, appPackage, target, userArgs); return; diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index b1dc990f5..cca3d0322 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -673,7 +673,7 @@ agent-device settings permission reset screen-recording --platform macos - macOS permission targets: `accessibility`, `screen-recording`, `input-monitoring`. - On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions. - On macOS, `settings permission deny ...` is intentionally unsupported. -- Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. Every permission mutation names the foreground user explicitly (`--user `, resolved with `am get-current-user`): `pm` defaults these operations to user 0, so on a device whose foreground user is nonzero an unscoped revoke would edit user 0 and leave the running app's permission untouched. +- Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications. Every permission mutation names the foreground user explicitly (`--user `, resolved with `am get-current-user`): `pm` defaults these operations to user 0, so on a device whose foreground user is nonzero an unscoped revoke would edit user 0 and leave the running app's permission untouched. Resolving that user is a prerequisite — if `am get-current-user` does not answer, `settings permission` fails with `COMMAND_FAILED` and changes nothing rather than applying the mutation to user 0. - Android kills a running app whenever a runtime permission it currently holds is revoked, so `settings permission deny|reset` after a grant leaves the session app no longer running. The response reports the prior state of the revoked permission for the acting user as `priorGrantState: granted | not_granted | unknown`, and carries a warning naming `open --relaunch` for both `granted` and `unknown` — `unknown` means the device did not report a readable state, not that the app was left alone. Revoking a permission the app does not hold (`not_granted`) is harmless and warns nothing. - `full|limited` mode is supported only for iOS `photos`; other targets reject mode. - Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes.