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
9 days ago

Closes #209.

What was wrong

On Android the enable* flags decided what went into AndroidManifest.xml and what window.craft.capabilities reported, and nothing else. Only push was actually refused, through a literal rejection the generator wrote. So an app built with enableShare: false told the page capabilities.share === false and then shared anyway, while the same config on iOS rejects with CAPABILITY_DISABLED in both Swift and Zig.

Where the gate goes, and why there

A gate over the page API, installed right after window.craft is built and before the v1 contract wraps it, so craft.share.share() refuses too.

It reads the same craft.capabilities object the page reads. That matters more than where the check runs: the two can't drift, because there is only one answer. The alternative the issue sketched — a config parser in Zig plus the same checks in Kotlin, mirroring iOS — is a bigger change for a smaller gain here, because Android's call path is the other way round: the page calls Kotlin, which calls Zig. There is no path where Zig sees a call the page did not make.

Kotlin checks haptics, speechRecognition, share and keepAwake a second time, from the same generated flags. Those are the ones Android itself does not gate with a permission, so a caller reaching CraftAndroid directly would otherwise still buzz, listen, share or hold the screen awake. Everything else needs a runtime permission the manifest no longer declares when its flag is off.

What a refused call answers

kind of callanswer
returns a promise (share, openCamera, authenticate, registerPush, getCurrentPosition, secureStore.*, notifications.*, deepLinks.getInitialURL, …)rejects with code: 'CAPABILITY_DISABLED'
answers nothing yet (haptic, vibrate, startListening, setKeepAwake, watchPosition, setFlashlight, …)does nothing, and says so on the console

The second row is #219: those calls return undefined on Android, so there is no promise to reject. When #219 gives them one, they move to the first row without touching the gate.

This is a behaviour change

Every enable* flag defaults to off. An app that never set them and relied on Android serving the call anyway will now be refused, and has to turn the capability on. That is deliberate:

  • it is what craft.capabilities has been telling those pages all along;
  • it is what the same craft.config.json already does on iOS;
  • and the manifest already reflected the flags, so anything needing a permission was failing at the permission check with an Android-specific message rather than with the code that says the app was built without it.

It is called out in packages/android/README.md.

Tests

  • packages/android/src/page-script.test.ts runs the real injected script under Bun, with capabilities on and off:
    • refused with the code, through both craft.share() and the v1 craft.share.share(), and native is never called;
    • haptic and vibrate do nothing rather than throwing, and native is never called;
    • with the flags on, the same calls reach native.
    • Not mutation-checked, unlike the other tests here: an ungated refused call never settles in this harness, which is the old behaviour rather than a failure the runner can report.
  • packages/android/src/index.test.ts pins the generated Kotlin: the gate is installed, it reads craft.capabilities, and the flags come from this app's config (shareEnabled = true, hapticsEnabled = false) with the native checks in place.
  • E2E: the Android probe now enables sharing, so the share cases still run, and leaves push and secure storage off. push.disabled.rejects checks the code rather than the generated message, and a new capabilities.match.behaviour case asserts that what capabilities reports and what the call does agree, both ways.

Not verified locally

No Android toolchain on this machine: the Kotlin compile (android-builder) and the emulator legs are checked only by this PR's CI.

Locally: bun run test, bun run typecheck, bun run verify:packages, Pickier, the android/ios/typescript package tests and 55 protocol tests.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.