ReviewOS

also looking at this

craft-native/craft

fix(android): enforce the capability flags the bridge already reports

#233
Merged glennmichael123 wants to merge fix/android-capability-gate into main
7 files +187 -10
packages/android/README.mdmodified+8-0
Changes to packages/android/README.md
@@ -132,6 +132,14 @@ Edit `craft.config.json` in your Android project:
132132}
133133```
134134
135Every `enable*` flag is enforced, not only reported. A call whose capability is
136off rejects with `CAPABILITY_DISABLED`, as it does on iOS, and
137`window.craft.capabilities` reports the same answer, so a page that
138feature-detects and a page that just calls behave the same. Calls that answer
139nothing yet, such as `haptic` and `vibrate`, do nothing and say so on the
140console. Every flag defaults to off, so an app that shipped without setting one
141and relied on Android serving it anyway has to turn it on.
142
135143Only the fields shown above are generator options. Enabling background location
136144also enables foreground geolocation. Push notifications require a regular
137145`googleServicesFile` whose JSON contains a client matching the generated
packages/android/src/index.test.tsmodified+18-0
Changes to packages/android/src/index.test.ts
@@ -359,6 +359,24 @@ describe('Craft Android builder', () => {
359359 expect(holder).toContain('private external fun nativeDispatchDeepLink(url: String, initial: Boolean): Boolean')
360360 })
361361
362 it('enforces the capability flags it reports, in the page and in Kotlin', async () => {
363 // #209: the flags shaped the manifest and craft.capabilities, and then
364 // every call was served anyway.
365 const output = mkdtempSync(join(tmpdir(), 'craft-android-capability-gate-'))
366 await init({ name: 'WildLoop', packageName: 'org.wildloop.app', output, config: { enableShare: true } })
367
368 const bridge = readFileSync(join(output, 'app/src/main/java/org/wildloop/app/CraftBridge.kt'), 'utf8')
369 expect(bridge).toContain('installCapabilityGates')
370 expect(bridge).toContain("error.code = 'CAPABILITY_DISABLED'")
371 // The gate reads the object the page reads, so the two cannot disagree.
372 expect(bridge).toContain('if (craft.capabilities[capability]) { return served.apply(this, arguments); }')
373 // Generated from this app's config, not hardcoded.
374 expect(bridge).toContain('private val shareEnabled = true')
375 expect(bridge).toContain('private val hapticsEnabled = false')
376 expect(bridge).toContain('if (!hapticsEnabled && disabled("haptics", "haptic")) return')
377 expect(bridge).toContain('if (!keepAwakeEnabled && disabled("keepAwake", "setKeepAwake")) return false')
378 })
379
362380 it('routes external Activity results back to every pending media promise', async () => {
363381 const output = mkdtempSync(join(tmpdir(), 'craft-android-activity-results-'))
364382 await init({ name: 'WildLoop', packageName: 'org.wildloop.app', output })
packages/android/src/page-script.test.tsmodified+42-5
Changes to packages/android/src/page-script.test.ts
@@ -9,8 +9,8 @@ import { renderAndroidPromiseRuntime } from './promise-runtime'
99
1010const template = readFileSync(join(import.meta.dir, '../templates/CraftBridge.kt.template'), 'utf8')
1111
12/** The page script, as Kotlin evaluates it, with every flag off. */
13function pageScript(): string {
12/** The page script, as Kotlin evaluates it, with every capability on or off. */
13function pageScript(enabled = true): string {
1414 const opening = ' val script = """\n'
1515 const start = template.indexOf(opening)
1616 if (start === -1) throw new Error('CraftBridge.kt.template no longer builds `val script = """`')
@@ -18,7 +18,7 @@ function pageScript(): string {
1818 const script = template
1919 .slice(start + opening.length, end)
2020 .replace(/\{\{PROMISE_RUNTIME\}\}/g, () => renderAndroidPromiseRuntime(' '))
21 .replace(/\{\{ENABLE_[A-Z_]+\}\}/g, 'false')
21 .replace(/\{\{ENABLE_[A-Z_]+\}\}/g, String(enabled))
2222 // Kotlin string templates, `${isBiometricAvailable()}` and the like. Each
2323 // is a Boolean in the capabilities object.
2424 .replace(/\$\{(?:[^{}]|\{[^{}]*\})*\}/g, 'false')
@@ -29,7 +29,7 @@ function pageScript(): string {
2929type Listener = (event: { type: string, detail: unknown }) => void
3030
3131/** `beforeInject` runs first, the way a page's own script runs before Android injects the bridge. */
32function loadPage(beforeInject?: (page: Record<string, any>) => void) {
32function loadPage(beforeInject?: (page: Record<string, any>) => void, enabled = true) {
3333 const calls: string[] = []
3434 const listeners: Record<string, Listener[]> = {}
3535 const page: Record<string, any> = {
@@ -66,7 +66,7 @@ function loadPage(beforeInject?: (page: Record<string, any>) => void) {
6666
6767 const inject = () => {
6868 // eslint-disable-next-line no-new-func
69 new Function('window', 'document', 'CraftAndroid', 'CustomEvent', 'console', pageScript())(
69 new Function('window', 'document', 'CraftAndroid', 'CustomEvent', 'console', pageScript(enabled))(
7070 page,
7171 document,
7272 CraftAndroid,
@@ -197,6 +197,43 @@ describe('the injected Android page script', () => {
197197 expect(seen).toEqual([{ url: 'app://warm', initial: false }])
198198 })
199199
200 // #209: the flags craft.capabilities reports are enforced, as on iOS.
201 it('refuses a call whose capability the app was not built with', async () => {
202 const page = loadPage(undefined, false)
203
204 expect(page.craft.capabilities.share).toBe(false)
205 await expect(page.craft.share('hello')).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' })
206 await expect(page.craft.deepLinks.getInitialURL()).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' })
207 await expect(page.craft.secureStore.set('k', 'v')).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' })
208 expect(page.calls).toEqual([])
209 }, 5000)
210
211 it('refuses through the v1 contract as well, since it wraps the same call', async () => {
212 const page = loadPage(undefined, false)
213 await expect(page.craft.share.share({ text: 'hello' })).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' })
214 }, 5000)
215
216 it('does nothing, rather than refusing, for a call with no answer to give', async () => {
217 // haptic and vibrate still return undefined (#219), so there is no
218 // promise to reject; native must not be asked to buzz either.
219 const page = loadPage(undefined, false)
220
221 expect(page.craft.haptic('light')).toBeUndefined()
222 expect(page.craft.vibrate([100])).toBeUndefined()
223 expect(page.calls).toEqual([])
224 })
225
226 it('serves the same calls when the app was built with them', async () => {
227 const page = loadPage()
228
229 expect(page.craft.capabilities.share).toBe(true)
230 void page.craft.share('hello')
231 void page.craft.secureStore.set('k', 'v')
232 page.craft.haptic('light')
233
234 expect(page.calls).toEqual(['share', 'secureSet', 'haptic'])
235 })
236
200237 it('does not hand a link to a second subscriber after the script runs again in the same page', async () => {
201238 const page = loadPage()
202239 const first: unknown[] = []
packages/android/templates/CraftBridge.kt.templatemodified+87-0
Changes to packages/android/templates/CraftBridge.kt.template
@@ -221,6 +221,20 @@ class CraftBridge(
221221 private var bridgeReady = false
222222 private val pendingEvents = mutableListOf<Pair<String, Map<String, Any>>>()
223223
224 // What the app was built with (#209). The page refuses a disabled call
225 // before it gets here; these are the second check, for a caller that
226 // reaches CraftAndroid directly, on the capabilities Android itself does
227 // not gate with a permission.
228 private val hapticsEnabled =
229 private val speechEnabled =
230 private val shareEnabled =
231 private val keepAwakeEnabled =
232
233 private fun disabled(capability: String, call: String): Boolean {
234 android.util.Log.d("CraftBridge", "$call refused: $capability is disabled in craft.config.json")
235 return true
236 }
237
224238 // Deep links (#215), on the UI thread only, the way iOS's DeepLinkManager
225239 // holds them: a link that arrives before the first page has its bridge is
226240 // the launch link, and every link waits for an injected bridge.
@@ -1367,6 +1381,70 @@ class CraftBridge(
13671381 }
13681382 };
13691383
1384 // What the app was built with, enforced and not just reported
1385 // (#209). iOS refuses a call whose capability is off with
1386 // CAPABILITY_DISABLED, in Swift and in Zig; Android shaped the
1387 // manifest and craft.capabilities from the same flags and then
1388 // served the call anyway, so a page that feature-detected and a
1389 // page that just called got different behaviour.
1390 //
1391 // The refusal is made here, from the same capabilities object the
1392 // page can read, because that object is what this bridge promises.
1393 // Calls that answer nothing yet (#219) cannot carry a refusal, so
1394 // they do nothing and say so on the console, which is what a
1395 // capability the app never asked for should do.
1396 //
1397 // Kotlin refuses the ungated-by-Android ones a second time, for a
1398 // caller that reaches CraftAndroid without going through here.
1399 (function installCapabilityGates(craft) {
1400 var gated = [
1401 ['haptic', 'haptics'],
1402 ['vibrate', 'haptics'],
1403 ['startListening', 'speechRecognition'],
1404 ['stopListening', 'speechRecognition'],
1405 ['setFlashlight', 'flashlight'],
1406 ['toggleFlashlight', 'flashlight'],
1407 ['setKeepAwake', 'keepAwake'],
1408 ['watchPosition', 'geolocation'],
1409 ['clearWatch', 'geolocation'],
1410 ['share', 'share', true],
1411 ['openCamera', 'camera', true],
1412 ['pickImage', 'camera', true],
1413 ['startVideoRecording', 'camera', true],
1414 ['authenticate', 'biometric', true],
1415 ['registerPush', 'pushNotifications', true],
1416 ['getCurrentPosition', 'geolocation', true],
1417 ['secureStore.set', 'secureStorage', true],
1418 ['secureStore.get', 'secureStorage', true],
1419 ['secureStore.remove', 'secureStorage', true],
1420 ['notifications.schedule', 'localNotifications', true],
1421 ['notifications.cancel', 'localNotifications', true],
1422 ['notifications.cancelAll', 'localNotifications', true],
1423 ['deepLinks.getInitialURL', 'deepLinks', true]
1424 ];
1425 function refusal(capability) {
1426 var error = new Error(capability + ' is disabled: this app was built without it');
1427 error.code = 'CAPABILITY_DISABLED';
1428 error.bridge = true;
1429 return error;
1430 }
1431 gated.forEach(function(entry) {
1432 var path = entry[0].split('.');
1433 var capability = entry[1];
1434 var awaited = entry[2] === true;
1435 var owner = path.length > 1 ? craft[path[0]] : craft;
1436 var name = path[path.length - 1];
1437 if (!owner || typeof owner[name] !== 'function') { return; }
1438 var served = owner[name];
1439 owner[name] = function() {
1440 if (craft.capabilities[capability]) { return served.apply(this, arguments); }
1441 if (awaited) { return Promise.reject(refusal(capability)); }
1442 console.warn('craft: ' + name + ' did nothing because ' + capability + ' is disabled');
1443 return undefined;
1444 };
1445 });
1446 })(window.craft);
1447
13701448 // Links that arrive before anything is listening (#215, as iOS
13711449 // does since #198).
13721450 //
@@ -1571,6 +1649,7 @@ class CraftBridge(
15711649
15721650 @JavascriptInterface
15731651 fun haptic(style: String) {
1652 if (!hapticsEnabled && disabled("haptics", "haptic")) return
15741653 if (CraftNative.haptic(activity, style)) return
15751654
15761655 val vibrator = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
@@ -1603,6 +1682,7 @@ class CraftBridge(
16031682
16041683 @JavascriptInterface
16051684 fun startListening() {
1685 if (!speechEnabled && disabled("speechRecognition", "startListening")) return
16061686 if (CraftNative.startListening(activity)) return
16071687
16081688 if (!SpeechRecognizer.isRecognitionAvailable(activity)) {
@@ -1711,6 +1791,11 @@ class CraftBridge(
17111791
17121792 @JavascriptInterface
17131793 fun share(text: String, title: String) {
1794 if (!shareEnabled) {
1795 rejectShare("share is disabled: this app was built without it")
1796 disabled("share", "share")
1797 return
1798 }
17141799 // The same line iOS draws: an empty string is nothing to share, and a
17151800 // chooser opened over nothing would offer apps a blank message.
17161801 if (text.isEmpty()) {
@@ -2743,6 +2828,7 @@ class CraftBridge(
27432828
27442829 @JavascriptInterface
27452830 fun vibrate(patternJson: String) {
2831 if (!hapticsEnabled && disabled("haptics", "vibrate")) return
27462832 if (CraftNative.vibrate(activity, patternJson)) return
27472833
27482834 val vibrator = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
@@ -3342,6 +3428,7 @@ class CraftBridge(
33423428
33433429 @JavascriptInterface
33443430 fun setKeepAwake(enabled: Boolean): Boolean {
3431 if (!keepAwakeEnabled && disabled("keepAwake", "setKeepAwake")) return false
33453432 // Zig queues the same window call on the same looper. isKeepingAwake
33463433 // below is written here and read nowhere, so the Kotlin body not
33473434 // running leaves nothing stale — unlike the flashlight pair.
scripts/mobile-e2e/android.tsmodified+5-0
Changes to scripts/mobile-e2e/android.ts
@@ -27,7 +27,12 @@ import { command, driverPage, waitForFile } from './support'
2727const DEEP_LINK_SCHEME = 'crafte2eprobe'
2828
2929const CONFIG = {
30 // Off, so the capability gate has something to refuse (#209), and on, so it
31 // has something to serve: the share cases below need sharing enabled now
32 // that the flag is enforced.
3033 enablePushNotifications: false,
34 enableSecureStorage: false,
35 enableShare: true,
3136 enableGeolocation: true,
3237 enableDeepLinks: true,
3338 urlSchemes: [DEEP_LINK_SCHEME],
scripts/mobile-e2e/driver.htmlmodified+26-5
Changes to scripts/mobile-e2e/driver.html
@@ -355,17 +355,38 @@
355355 }
356356 },
357357 {
358 // Android's equivalent gate. With push disabled the generator emits a
359 // literal reject call rather than a Firebase block, so the rejection
360 // string is fixed at generation time and is safe to match on.
358 // Android's equivalent gate, and since #209 the same code iOS sends:
359 // push is off in this app's config, so the call is refused rather
360 // than reaching Firebase. capabilities says so too, and the case
361 // checks that the two agree.
361362 name: 'push.disabled.rejects',
362363 platforms: ['android'],
363364 run: function () {
365 if (window.craft.capabilities.pushNotifications !== false)
366 return 'capabilities.pushNotifications is ' + JSON.stringify(window.craft.capabilities.pushNotifications)
364367 return withTimeout(function () { return window.craft.registerPush() }).then(function (outcome) {
365368 if (outcome.state === 'timeout') return 'registerPush never settled'
366369 if (outcome.state === 'resolved') return 'registerPush resolved while push is disabled'
367 if (outcome.error.message.indexOf('Push notifications are disabled') === -1)
368 return 'message is ' + JSON.stringify(outcome.error.message)
370 if (outcome.error.code !== 'CAPABILITY_DISABLED') return 'code is ' + JSON.stringify(outcome.error.code)
371 })
372 }
373 },
374 {
375 // #209: a capability the app was built with is served, and the page
376 // can tell which is which from capabilities alone. Sharing is on in
377 // this app precisely so the refusal above is not the only answer the
378 // gate can give.
379 name: 'capabilities.match.behaviour',
380 platforms: ['android'],
381 run: function () {
382 if (window.craft.capabilities.share !== true)
383 return 'capabilities.share is ' + JSON.stringify(window.craft.capabilities.share)
384 if (window.craft.capabilities.secureStorage !== false)
385 return 'capabilities.secureStorage is ' + JSON.stringify(window.craft.capabilities.secureStorage)
386 return withTimeout(function () { return window.craft.secureStore.set('craft-e2e', NONCE) }).then(function (outcome) {
387 if (outcome.state === 'timeout') return 'secureStore.set never settled'
388 if (outcome.state === 'resolved') return 'secureStore.set resolved while secure storage is disabled'
389 if (outcome.error.code !== 'CAPABILITY_DISABLED') return 'code is ' + JSON.stringify(outcome.error.code)
369390 })
370391 }
371392 }
scripts/mobile-e2e/protocol.tsmodified+1-0
Changes to scripts/mobile-e2e/protocol.ts
@@ -96,6 +96,7 @@ export const REQUIRED_CASES: Record<MobilePlatform, string[]> = {
9696 'share.empty.rejects',
9797 'share.dismissed.resolvesFalse',
9898 'permissions.location.granted',
99 'capabilities.match.behaviour',
99100 ],
100101}
101102