feat: revalidate device permissions against OS state before use - #471
feat: revalidate device permissions against OS state before use#471TarikGul wants to merge 11 commits into
Conversation
Regenerates the host-callback surface: permissionStatus is an optional capability group, so a host that omits it is unchanged.
Amends RFC-0002's permission lifecycle, the protocol doc comment on HostDevicePermissionRequest, and the platform/host READMEs.
…ment The doc comment on HostDevicePermissionRequest reaches the truapi namespace bindings, so the committed Swift carries it too.
Imod7
left a comment
Imod7
left a comment
There was a problem hiding this comment.
In js/packages/truapi-host/src/web/create-worker-host-runtime.ts, line 833:
capabilities: { chat: host.chat !== undefined },
This never reports permissionStatus, so the worker does not proxy devicePermissionStatus and no revalidation happens in the browser. I tested it: the init message carries {"chat":false} even when the host serves the capability. Please add permissionStatus: host.permissionStatus !== undefined. The two toEqual assertions on capabilities in worker-provider.test.ts then need the new key, and makeHostCallbacks in test-support.ts needs a permissionStatus branch like the chat one.
| // A stored answer stands, except where the OS has forgotten its own | ||
| // grant. That is the one case a prompt resolves rather than repeats: | ||
| // it reaches the OS dialog instead of re-asking the user. | ||
| let os_forgot_its_grant = os == DevicePermissionStatus::NotDetermined |
There was a problem hiding this comment.
This branch never converges. If the prompt does not move the OS out of NotDetermined, the stored answer stays Authorized, the OS answer stays NotDetermined, and every request re-prompts. I tested it: three calls, three prompts. RFC 0002 step 3 and the doc on HostDevicePermissionRequest both say the user is not asked again.
| // it reaches the OS dialog instead of re-asking the user. | ||
| let os_forgot_its_grant = os == DevicePermissionStatus::NotDetermined | ||
| && cached == StoredAuthorizationStatus::Authorized; | ||
| if !os_forgot_its_grant { |
There was a problem hiding this comment.
Falling through here prompts and then persists the answer, so a stored product grant is replaced by the prompt's result. On iOS and Android that prompt is the OS dialog, so a "Don't Allow" becomes a permanent product denial: restoring the capability in system settings gives Granted from the OS and Denied from storage, this branch stops firing, and nothing asks again. A prompt error on the same path returns NotDetermined, so a valid grant reads as granted: false.
pgherveou
left a comment
pgherveou
left a comment
There was a problem hiding this comment.
approving to unblock, will give a more torough review later this evening
| Chat calls `Unsupported` otherwise. Codegen reads `OptionalPlatform` to emit | ||
| each listed capability as an optional group on the host-callback surface. | ||
| traits above except `ChatPlatform` and `PermissionStatusHost`, which | ||
| `OptionalPlatform` lists instead: a host supplies each only when it can serve |
There was a problem hiding this comment.
do we want to treat Permission the same way as Chat ?
not really up to date here and not sure why we have OptionalPlatform in the first place rather than defaulting to an impl that return unsupported
| /// its own, so this is how the revalidation path is driven from a script. | ||
| /// | ||
| /// [`Display`]: core::fmt::Display | ||
| os_denied_permissions: BTreeSet<String>, |
There was a problem hiding this comment.
do we need that rather than just hard coding unsupported in the implementation?
| #[async_trait] | ||
| pub trait PermissionStatusHost: Send + Sync { | ||
| /// Current OS status of a device capability. Must not prompt. | ||
| async fn device_permission_status( |
There was a problem hiding this comment.
I thought we already have thad defined, how where we handling device permission before 🤔
An undetermined OS status no longer triggers a prompt. The prompt callback answers the product's question as well as the OS one, so using it to reach the OS dialog re-asked an answered question on every request and persisted the OS answer over the product's decision. An OS dialog declined that way became a permanent product denial that restoring the capability in system settings could not recover, and a transient prompt failure on the same path reported a held grant as denied. The OS resolves its own gate when the capability is used, so the core defers to the stored decision unless the OS refuses outright. Also reports permissionStatus in the Web Worker init message, without which the worker proxies no status callback and nothing revalidates in a browser.
3 participants
The decision rule
The two gates are combined, not substituted:
The host admin surface (
CoreAdmin::get_permission_authorization_status) keeps reporting stored state on its own. It exists to show what the user decided about a product, and folding OS state into it would make that reading wrong.Hosts
truapi-host-cliserves the capability, driven byTRUAPI_OS_DENIED_PERMISSIONS(comma-separated capability names, matched case-insensitively). Capabilities it does not name reportNotApplicablerather thanGranted, since a terminal has no OS gate to grant anything.The WASM hosts install the adapter when the JS side supplies
devicePermissionStatus.Not in scope
Native hosts cannot answer yet:
HostCallbackshas no status method, so iOS and Android adoption is a follow-up on each. Until then they resolve from stored state, exactly as now.granted: falsestill covers both a product-scoped refusal and an OS-level one, so a product cannot tell "you declined this" from "the OS revoked this, open system settings". The two have different remedies. Distinguishing them needs a richer device-permission response than the current boolean, so it waits for a breaking-change window.Verification
Nine unit tests cover the decision matrix in
host_logic::permissions. Three integration tests drive a realpermissions_request_device_permissionframe through the generated dispatcher, which is the only place the plumbing from installer to runtime to service is exercised.Every one of those tests was confirmed to fail under a deliberate mutation: dropping the OS-denial override, never re-prompting after a reset, mapping a query failure to a refusal, clearing the stored grant on refusal, re-prompting on any
NotDetermined, treating an absent adapter as a refusal, an installer that stores nothing, and a runtime that builds the service without the adapter.Live, against the
truapi-hostCLI with persisted state across processes:TRUAPI_OS_DENIED_PERMISSIONSgranted: truegranted: truegranted: truegranted: truecameragranted: falsegranted: truegranted: truegranted: truegranted: truegranted: truegranted: falsegranted: falseC is the fix: a persisted grant stops answering while the OS refuses it, and only for the capability named. E shows the refusal did not erase the grant. F and G separate stored state from prompting: with no auto-accept and stdin closed, warm state still answers
trueand cold state cannot.rel: #334