Skip to content

Commit a5a8a0b

Browse files
committed
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 <app> --relaunch; the settings CLI output renders response warnings, and commands.md documents the behavior next to the pm revoke mapping. Closes #1796
1 parent fda81c5 commit a5a8a0b

6 files changed

Lines changed: 253 additions & 25 deletions

File tree

‎src/commands/capture/index.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { alertCliReader, alertDaemonWriter } from './alert.ts';
44
import { diffCliReader } from './diff.ts';
55
import { snapshotCliOutput } from './output.ts';
66
import { screenshotCliReader, screenshotDaemonWriter } from './screenshot.ts';
7-
import { settingsCliReader, settingsDaemonWriter } from './settings.ts';
7+
import { settingsCliReader, settingsCommandFacet, settingsDaemonWriter } from './settings.ts';
88
import { snapshotCliReader } from './snapshot.ts';
99
import { waitCliReader, waitDaemonWriter } from './wait.ts';
1010

@@ -187,4 +187,26 @@ describe('capture command interface', () => {
187187
positionals: ['permission', 'grant', 'camera', 'limited'],
188188
});
189189
});
190+
191+
// #1796: the Android revoke warning rides `warnings`; the human CLI line must show it.
192+
test('settings CLI output renders response warnings after the message', () => {
193+
const warning = 'Revoking android.permission.CAMERA while it was granted made Android kill …';
194+
const output = settingsCommandFacet.cliOutputFormatter!({
195+
input: {},
196+
result: { setting: 'permission', state: 'reset', message: 'Updated setting: permission' },
197+
});
198+
expect(output.text).toBe('Updated setting: permission');
199+
200+
const warned = settingsCommandFacet.cliOutputFormatter!({
201+
input: {},
202+
result: {
203+
setting: 'permission',
204+
state: 'reset',
205+
message: 'Updated setting: permission',
206+
warnings: [warning],
207+
},
208+
});
209+
expect(warned.text).toBe(`Updated setting: permission\nWarning: ${warning}`);
210+
expect(warned.data).toMatchObject({ warnings: [warning] });
211+
});
190212
});

‎src/commands/capture/settings.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import type { CliReader, DaemonWriter } from '../cli-grammar/types.ts';
1818
import { defineCommandFacet } from '../family/types.ts';
1919
import { defineFieldCommandMetadata } from '../field-command-contract.ts';
20+
import { messageWithWarningsOutput } from '../output-common.ts';
2021

2122
const SETTINGS_COMMAND_NAME = 'settings';
2223
const settingsCommandDescription =
@@ -59,13 +60,15 @@ export const settingsCommandFacet = defineCommandFacet({
5960
text: {
6061
summary: 'Change OS settings and app permissions',
6162
cliDetail:
62-
'macOS supports only settings appearance <light|dark|toggle> and settings permission <grant|reset> <accessibility|screen-recording|input-monitoring>; wifi|airplane|location|animations remain unsupported on macOS. Mobile permission actions use the active session app.',
63+
'macOS supports only settings appearance <light|dark|toggle> and settings permission <grant|reset> <accessibility|screen-recording|input-monitoring>; 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 <app> --relaunch restores it.',
6364
},
6465
metadata: settingsCommandMetadata,
6566
definition: settingsCommandDefinition,
6667
cliSchema: settingsCliSchema,
6768
cliReader: settingsCliReader,
6869
daemonWriter: settingsDaemonWriter,
70+
// Android permission revokes append a relaunch warning (#1796); render it for humans too.
71+
cliOutputFormatter: messageWithWarningsOutput,
6972
});
7073

7174
// fallow-ignore-next-line complexity

‎src/commands/output-common.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,23 @@ export function messageCliOutput(result: Record<string, unknown>): CliOutput {
1818
return { data: result, text: readCommandMessage(result) };
1919
}
2020

21+
/**
22+
* `messageCliOutput` plus one `Warning:` line per entry of the response's `warnings`
23+
* array — the composable warnings channel (`open`, `debug`, snapshot capture use it too),
24+
* so a warning the daemon appended reaches the human CLI reader, not only `--json`.
25+
*/
26+
export const messageWithWarningsOutput = resultOutput(
27+
(result: Record<string, unknown>): CliOutput => {
28+
const output = messageCliOutput(result);
29+
const warnings = Array.isArray(result.warnings)
30+
? result.warnings.filter((warning): warning is string => typeof warning === 'string')
31+
: [];
32+
if (warnings.length === 0) return output;
33+
const lines = [output.text, ...warnings.map((warning) => `Warning: ${warning}`)];
34+
return { data: output.data, text: lines.filter(Boolean).join('\n') };
35+
},
36+
);
37+
2138
/**
2239
* ADR 0014: a reusable ref in a PARTIAL result renders in ready-to-copy
2340
* `@eN~s<refsGeneration>` form so a human CLI caller can paste it into the next

‎src/platforms/android/__tests__/settings.test.ts‎

Lines changed: 105 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { test } from 'vitest';
22
import assert from 'node:assert/strict';
3-
import { setAndroidSetting } from '../settings.ts';
3+
import {
4+
androidRevokedGrantedPermissionWarning,
5+
parseAndroidGrantedRuntimePermissions,
6+
setAndroidSetting,
7+
} from '../settings.ts';
48
import {
59
ANDROID_EMULATOR,
610
assertRejectsAppError,
@@ -177,6 +181,106 @@ test('setAndroidSetting permission reset notifications clears permission flags f
177181
);
178182
});
179183

184+
// #1796: Android kills the app when a permission it holds is revoked. The prior grant state is
185+
// read before `pm revoke`, and a granted -> revoked transition surfaces as a warning + typed field.
186+
const DUMPSYS_MICROPHONE_GRANTED = [
187+
'Packages:',
188+
' Package [com.example.app] (abc):',
189+
' User 0: ceDataInode=0 installed=true',
190+
' runtime permissions:',
191+
' android.permission.RECORD_AUDIO: granted=true, flags=[ USER_SET|USER_SENSITIVE_WHEN_GRANTED]',
192+
' android.permission.CAMERA: granted=false, flags=[ USER_SENSITIVE_WHEN_GRANTED]',
193+
].join('\n');
194+
195+
test.each(['deny', 'reset'] as const)(
196+
'setAndroidSetting permission %s warns that revoking a granted permission killed the app',
197+
async (action) => {
198+
await withFakeAdb(
199+
(args) =>
200+
args.join(' ') === 'shell dumpsys package com.example.app'
201+
? DUMPSYS_MICROPHONE_GRANTED
202+
: undefined,
203+
async ({ calls, device }) => {
204+
const result = await setAndroidSetting(device, 'permission', action, 'com.example.app', {
205+
permissionTarget: 'microphone',
206+
});
207+
const flat = calls.map((args) => args.join(' '));
208+
// The state is read BEFORE the revoke: after it, dumpsys would already say false.
209+
assert.ok(
210+
flat.indexOf('shell dumpsys package com.example.app') <
211+
flat.indexOf('shell pm revoke com.example.app android.permission.RECORD_AUDIO'),
212+
flat.join('; '),
213+
);
214+
assert.deepEqual(result, {
215+
permission: 'android.permission.RECORD_AUDIO',
216+
wasGranted: true,
217+
warnings: [
218+
androidRevokedGrantedPermissionWarning(
219+
'com.example.app',
220+
'android.permission.RECORD_AUDIO',
221+
),
222+
],
223+
});
224+
assert.match(String(result?.warnings), /open com\.example\.app --relaunch/);
225+
},
226+
);
227+
},
228+
);
229+
230+
test('setAndroidSetting permission deny stays quiet when the permission was not granted', async () => {
231+
await withFakeAdb(
232+
(args) =>
233+
args.join(' ') === 'shell dumpsys package com.example.app'
234+
? DUMPSYS_MICROPHONE_GRANTED
235+
: undefined,
236+
async ({ device }) => {
237+
const result = await setAndroidSetting(device, 'permission', 'deny', 'com.example.app', {
238+
permissionTarget: 'camera',
239+
});
240+
assert.deepEqual(result, { permission: 'android.permission.CAMERA', wasGranted: false });
241+
},
242+
);
243+
});
244+
245+
test('setAndroidSetting permission grant does not read grant state', async () => {
246+
await withFakeAdb(
247+
() => undefined,
248+
async ({ calls, device }) => {
249+
const result = await setAndroidSetting(device, 'permission', 'grant', 'com.example.app', {
250+
permissionTarget: 'camera',
251+
});
252+
assert.equal(result, undefined);
253+
assert.deepEqual(calls, [
254+
['shell', 'pm', 'grant', 'com.example.app', 'android.permission.CAMERA'],
255+
]);
256+
},
257+
);
258+
});
259+
260+
test('setAndroidSetting permission reset notifications warns when POST_NOTIFICATIONS was granted', async () => {
261+
await withFakeAdb(
262+
(args) =>
263+
args.join(' ') === 'shell dumpsys package com.example.app'
264+
? ' runtime permissions:\n android.permission.POST_NOTIFICATIONS: granted=true, flags=[ USER_SET]'
265+
: undefined,
266+
async ({ device }) => {
267+
const result = await setAndroidSetting(device, 'permission', 'reset', 'com.example.app', {
268+
permissionTarget: 'notifications',
269+
});
270+
assert.equal(result?.wasGranted, true);
271+
assert.equal(Array.isArray(result?.warnings), true);
272+
},
273+
);
274+
});
275+
276+
test('parseAndroidGrantedRuntimePermissions reads only granted=true runtime permissions', () => {
277+
assert.deepEqual(
278+
[...parseAndroidGrantedRuntimePermissions(DUMPSYS_MICROPHONE_GRANTED)],
279+
['android.permission.RECORD_AUDIO'],
280+
);
281+
assert.deepEqual([...parseAndroidGrantedRuntimePermissions('')], []);
282+
});
283+
180284
test('setAndroidSetting permission reset camera clears permission flags for reprompt', async () => {
181285
await withFakeAdb(
182286
() => undefined,

‎src/platforms/android/settings.ts‎

Lines changed: 103 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -142,34 +142,115 @@ export async function setAndroidSetting(
142142
if (!appPackage) {
143143
throw new AppError('INVALID_ARGS', 'permission setting requires an active app in session');
144144
}
145-
const action = parsePermissionAction(state);
146-
const target = parseAndroidPermissionTarget(
147-
options?.permissionTarget,
148-
options?.permissionMode,
149-
);
150-
if (target.kind === 'notifications') {
151-
await setAndroidNotificationPermission(device, appPackage, action, target);
152-
return;
153-
}
154-
const pmAction = action === 'grant' ? 'grant' : 'revoke';
155-
if (target.type === 'photos') {
156-
const permission = await setAndroidPhotoPermission(device, appPackage, pmAction);
157-
if (action === 'reset') {
158-
await clearAndroidPermissionFlags(device, appPackage, permission);
159-
}
160-
return;
161-
}
162-
await runAndroidAdb(device, ['shell', 'pm', pmAction, appPackage, target.value]);
163-
if (action === 'reset') {
164-
await clearAndroidPermissionFlags(device, appPackage, target.value);
165-
}
166-
return;
145+
return await setAndroidPermission(device, appPackage, state, options);
167146
}
168147
default:
169148
throw new AppError('INVALID_ARGS', `Unsupported setting: ${setting}`);
170149
}
171150
}
172151

152+
/**
153+
* Android kills the app's process whenever a runtime permission it currently holds is
154+
* revoked (`pm revoke` after a grant, foreground or background), so a `deny`/`reset` that
155+
* follows a grant leaves the session pointing at a dead app and the next selector fails
156+
* against the launcher (#1796). Revoking a permission the app does not hold is harmless.
157+
* The prior grant state is read before the revoke so the response can say so.
158+
*/
159+
export function androidRevokedGrantedPermissionWarning(
160+
appPackage: string,
161+
permission: string,
162+
): string {
163+
return (
164+
`Revoking ${permission} while it was granted made Android kill ${appPackage}; the app is ` +
165+
`no longer running. Relaunch it with open ${appPackage} --relaunch before the next interaction.`
166+
);
167+
}
168+
169+
type AndroidPermissionTarget = ReturnType<typeof parseAndroidPermissionTarget>;
170+
171+
async function setAndroidPermission(
172+
device: DeviceInfo,
173+
appPackage: string,
174+
state: string,
175+
options: SettingOptions | undefined,
176+
): Promise<Record<string, unknown> | void> {
177+
const action = parsePermissionAction(state);
178+
const target = parseAndroidPermissionTarget(options?.permissionTarget, options?.permissionMode);
179+
if (action === 'grant') {
180+
await grantAndroidPermission(device, appPackage, target);
181+
return;
182+
}
183+
const grantedBefore = await readAndroidGrantedRuntimePermissions(device, appPackage);
184+
const permission = await revokeAndroidPermission(device, appPackage, action, target);
185+
const wasGranted = grantedBefore.has(permission);
186+
return {
187+
permission,
188+
wasGranted,
189+
...(wasGranted
190+
? { warnings: [androidRevokedGrantedPermissionWarning(appPackage, permission)] }
191+
: {}),
192+
};
193+
}
194+
195+
async function grantAndroidPermission(
196+
device: DeviceInfo,
197+
appPackage: string,
198+
target: AndroidPermissionTarget,
199+
): Promise<void> {
200+
if (target.kind === 'notifications') {
201+
await setAndroidNotificationPermission(device, appPackage, 'grant', target);
202+
} else if (target.type === 'photos') {
203+
await setAndroidPhotoPermission(device, appPackage, 'grant');
204+
} else {
205+
await runAndroidAdb(device, ['shell', 'pm', 'grant', appPackage, target.value]);
206+
}
207+
}
208+
209+
/** Revokes (and for `reset`, clears the flags of) the target; returns the permission revoked. */
210+
async function revokeAndroidPermission(
211+
device: DeviceInfo,
212+
appPackage: string,
213+
action: 'deny' | 'reset',
214+
target: AndroidPermissionTarget,
215+
): Promise<string> {
216+
if (target.kind === 'notifications') {
217+
await setAndroidNotificationPermission(device, appPackage, action, target);
218+
return target.permission;
219+
}
220+
let permission: string;
221+
if (target.type === 'photos') {
222+
permission = await setAndroidPhotoPermission(device, appPackage, 'revoke');
223+
} else {
224+
permission = target.value;
225+
await runAndroidAdb(device, ['shell', 'pm', 'revoke', appPackage, permission]);
226+
}
227+
if (action === 'reset') await clearAndroidPermissionFlags(device, appPackage, permission);
228+
return permission;
229+
}
230+
231+
/**
232+
* Runtime permissions `dumpsys package <pkg>` reports as `granted=true`. Best-effort: a
233+
* failed or unparseable dump reads as "nothing granted", which only costs the warning.
234+
*/
235+
async function readAndroidGrantedRuntimePermissions(
236+
device: DeviceInfo,
237+
appPackage: string,
238+
): Promise<ReadonlySet<string>> {
239+
const result = await runAndroidAdb(device, ['shell', 'dumpsys', 'package', appPackage], {
240+
allowFailure: true,
241+
});
242+
if (result.exitCode !== 0) return new Set();
243+
return parseAndroidGrantedRuntimePermissions(result.stdout);
244+
}
245+
246+
export function parseAndroidGrantedRuntimePermissions(dumpsysOutput: string): ReadonlySet<string> {
247+
const granted = new Set<string>();
248+
for (const match of dumpsysOutput.matchAll(/^\s*([\w.]+): granted=true\b/gm)) {
249+
granted.add(match[1]!);
250+
}
251+
return granted;
252+
}
253+
173254
type AndroidFingerprintAction = 'match' | 'nonmatch';
174255

175256
function parseAndroidFingerprintAction(state: string): AndroidFingerprintAction {

‎website/docs/docs/commands.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,7 @@ agent-device settings permission reset screen-recording --platform macos
673673
- On macOS, `settings permission grant ...` checks/request access and opens System Settings guidance when needed; it does not silently grant TCC permissions.
674674
- On macOS, `settings permission deny ...` is intentionally unsupported.
675675
- Android uses `pm grant|revoke` for runtime permissions (`reset` maps to revoke) and `appops` for notifications.
676+
- 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 <app> --relaunch` before the next interaction. Revoking a permission the app does not hold is harmless.
676677
- `full|limited` mode is supported only for iOS `photos`; other targets reject mode.
677678
- Use `match`/`nonmatch` to simulate valid/invalid Face ID, Touch ID, and Android fingerprint outcomes.
678679

0 commit comments

Comments
 (0)