andodeki
cf878d9e3e
The API change tranche 10 scoped, in REVIEWS/adr/0007. ## dispatch_with_authorization Tranche 10 established that Android cannot implement BiometricAuthorizer without reopening defect S2, and built AuthorizationAttempt as the replacement. It named the remaining work: a coordinator entry point taking an already-granted attempt instead of calling biometric.authenticate(). coordinator.dispatch_with_authorization(&id, &mut attempt) It does not prompt, does not wait, and does not consult the injected BiometricAuthorizer at all. That is asserted rather than assumed: one test injects an authorizer reporting no hardware that also errors, and shows a granted attempt still dispatches. If the coordinator ever fell back to it, that test fails. Three refusals, each with a test that fails when the gate is removed: - An unanswered prompt does not dispatch. PromptShown plus SensorEngaged is not consent — the exact shape the Android adapter would produce. - A grant belongs to one payment. A foreign grant is refused before anything else, so a stray callback cannot even fail the payment on screen; the victim intent stays in AwaitingUserAuthorization rather than being transitioned to Failed by a stranger. - A grant is spent on use. One authorization, one dispatch (B3). Denied, still-prompting and already-spent attempts all fail the intent closed and never reach the gateway. ## Verifying the tests can fail The gate was removed (if !attempt.consume() -> if false) and the suite re-run: 5 tests failed. A fail-closed test that passes against fail-open code is worthless, so this is the check that matters. Worth recording: a_grant_cannot_dispatch_twice still passed with the gate removed, because the existing duplicate-dispatch budget caught it independently. Two unrelated mechanisms refuse the second dispatch. That is defence in depth working, and the reason that test is not sufficient evidence on its own. ## Validation domain : 137 tests --locked, fmt, clippy -D warnings, bench pass storage : 36 + 41 sqlcipher --locked, fmt, clippy pass platform: 56 + 64 ussd --locked, fmt, clippy, mock guard pass nigig-pay-ui: cargo test --lib pass (61) nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core: check pass authorization / batch / settlement-tick guards pass fail-open injection: 5 tests fail with the gate removed pass Domain tests 129 -> 137. ## What remains The authorization half is done and verified. PayFlowHandler still owns the USSD session lifecycle, the pending-store writes that shadow the coordinator's repository, and the bulk queue plumbing. Moving those makes the coordinator own the gateway session, which changes who cancels on teardown and who observes an out-of-order callback — ADR 0007's device matrix, which cannot be exercised here.
200 lines
9.4 KiB
Markdown
200 lines
9.4 KiB
Markdown
# ADR 0007: The payment platform boundary, and the AccessibilityService question
|
|
|
|
- **Status:** Accepted for the code; **Blocked** for the AccessibilityService rail
|
|
- **Date:** 2026-07-29
|
|
- **Review items:** 5.1, 5.2, 5.3, 5.4, 5.5, 5.6, 7.5 (Phase 5 and Phase 7,
|
|
`NIGIG_PAY_CONSOLIDATED_REVIEW.md`)
|
|
- **Related:** ADR 0001 (payment scope), ADR 0002 (crate ownership)
|
|
|
|
## Context
|
|
|
|
ADR 0002 named four layers and reserved a slot for `nigig-pay-platform`
|
|
described as "not yet created". Phase 5 is that crate.
|
|
|
|
Until now the platform boundary was not a boundary. `pay_flow_handler.rs`
|
|
imported `robius_ussd` directly, held session state in a `thread_local!`, and
|
|
matched inbound Android callbacks against whatever payment happened to be on
|
|
screen. Three consequences followed, and all three are ordinary on a real
|
|
device rather than exotic:
|
|
|
|
1. **Uncorrelated callbacks.** A result arriving from an abandoned session
|
|
settled whichever payment was current.
|
|
2. **Error kind mistaken for safety.** `TemporarilyUnavailable` was treated as
|
|
retryable regardless of whether the USSD menu had already been driven, so a
|
|
session that died after the transfer was submitted looked identical to one
|
|
that never dialled.
|
|
3. **Untrusted input treated as an outcome.** A scraped dialog or an SMS whose
|
|
sender field said `MPESA` was sufficient to mark a payment done.
|
|
|
|
## Decision
|
|
|
|
### 1. One crate owns the seam (5.1)
|
|
|
|
`nigig-pay-platform` is the only crate that may name a platform SDK. The
|
|
dependency direction is `domain ← storage ← platform ← ui`, and CI enforces it
|
|
in both directions: the payment crates may not import Makepad, and the domain
|
|
and storage crates may not import `jni`, `robius_ussd`, `robius_sms` or
|
|
`robius_fingerprinting`.
|
|
|
|
### 2. Correlation is mandatory, and single-flight (5.3)
|
|
|
|
`SessionRegistry` holds at most one live operation. Every inbound event is
|
|
checked against it and receives one of `Accepted`, `Duplicate` or `Ignored`.
|
|
There is no constructor for an uncorrelated event: `PlatformEvent` requires a
|
|
`CorrelationId`.
|
|
|
|
A closed session id is retired permanently. Reuse is refused, because the
|
|
alternative lets a late duplicate of an old callback land on a new payment.
|
|
|
|
### 3. Progress decides retry safety, not error kind (2.8, 5.3)
|
|
|
|
`classify_failure(failure, progress)` takes two inputs. `DispatchProgress`
|
|
is the authority:
|
|
|
|
| Progress | Meaning | Classification |
|
|
|---|---|---|
|
|
| `NotSent` | never left the device | by error kind |
|
|
| `MaybeSent` | may have reached the provider | always ambiguous |
|
|
| `Sent` | provider acknowledged | always ambiguous |
|
|
|
|
An explicit provider refusal is the single exception and stays a rejection at
|
|
any progress, because it is a statement rather than an absence.
|
|
|
|
Adapters must report progress honestly and must round *up* when unsure.
|
|
Over-reporting costs a reconciliation entry; under-reporting costs a duplicate
|
|
payment. `UssdGateway::note_progress` is monotonic so a late low-progress
|
|
event cannot downgrade a dispatch that already reached the provider.
|
|
|
|
### 4. A second, stricter SMS reader (D1, D2, 7.5)
|
|
|
|
`StrictMpesaSms` is deliberately *not* a replacement for `nigig-core`'s
|
|
parser, and this is not the duplication ADR 0002 forbids. They answer
|
|
different questions:
|
|
|
|
- `nigig-core` answers "what should the history screen show?" A message it
|
|
cannot read is a missing row, so it is permissive.
|
|
- `nigig-pay-platform` answers "may this influence a payment decision?" A
|
|
message it misreads is a reconciliation resolved on forged input, so it
|
|
refuses anything not unambiguously well-formed.
|
|
|
|
Concretely, the strict reader requires an exact 10-character upper-case
|
|
alphanumeric code containing both a letter and a digit, requires an exact
|
|
sender-ID match rather than a substring (`MPESA-REFUNDS` and `FAKE-MPESA` are
|
|
refused), rejects fractional shillings instead of rounding them, caps body
|
|
length, and refuses a message that states no outcome.
|
|
|
|
What it must never grow is a "close enough" path.
|
|
|
|
### 5. Fakes cannot ship (5.4, 0.5)
|
|
|
|
`MockGateway` exists only under `cfg(any(test, feature = "mock"))`. Enabling
|
|
`mock` in a release build is a `compile_error!` unless the operator also names
|
|
`allow-mock-in-release`. Every session id it issues is prefixed `MOCK-` so a
|
|
mock session that somehow reached a ledger is identifiable on sight. CI
|
|
asserts the release build genuinely fails.
|
|
|
|
### 6. The web claim is withdrawn (5.6, U3, Q13)
|
|
|
|
There is no browser API that can drive a USSD session, and a Daraja
|
|
credential must never be shipped to a browser. `WebGateway` therefore
|
|
implements the trait and refuses every call, mapping to `Fatal` — "never
|
|
sent" — which is accurate and owes no reconciliation. The refusal names the
|
|
Android app so the message is actionable.
|
|
|
|
### 7. No `unsafe`, and no PIN (5.5, 0.3)
|
|
|
|
The crate is `#![forbid(unsafe_code)]`, so the JNI surface stays in
|
|
`robius-ussd`. `UssdGateway` is `!Send`/`!Sync` by construction, which turns
|
|
the main-thread requirement into a compile error rather than a comment.
|
|
|
|
The adapter leaves `UssdTransactionRequest::pin` empty and a test asserts it.
|
|
This crate refuses to be the place a PIN is reintroduced.
|
|
|
|
## The AccessibilityService question — unresolved, and blocking
|
|
|
|
**Review item 5.2 requires a legal and policy review of the
|
|
AccessibilityService-driven USSD automation against current Google Play
|
|
policy. That review has not happened, and no amount of engineering in this
|
|
ADR substitutes for it.**
|
|
|
|
The engineering risk is now contained. The *existential* risk is not, and it
|
|
is not an engineering risk at all:
|
|
|
|
- Google Play's Accessibility API policy requires the API be used to help
|
|
users with disabilities, with a declared and user-visible purpose. Driving a
|
|
payment menu on the user's behalf is not that.
|
|
- Enforcement outcome is binary and retroactive: app removal, developer
|
|
account termination. A rewrite does not recover a terminated account.
|
|
- The rail is also technically brittle in a way no test suite fixes:
|
|
Safaricom can renumber a menu at any time, and the automation has no
|
|
contract entitling it to notice.
|
|
|
|
### What must happen before this rail is enabled
|
|
|
|
1. A named human accepts the Play-policy risk in writing, or the rail is
|
|
abandoned in favour of an authorised provider API (Daraja) with
|
|
server-side credentials.
|
|
2. Until then, USSD dispatch stays behind the default-off `demo` feature,
|
|
which is where it is today.
|
|
|
|
### Position if the review fails
|
|
|
|
ADR 0001 already contemplates this: the product is positioned as a
|
|
**tracker/launcher**, not a payment processor. The domain, storage and
|
|
platform crates were built so that outcome costs nothing — the observation
|
|
path, the evidence model and the reconciliation queue are all still correct
|
|
for a tracker. Only the dispatch adapter is discarded.
|
|
|
|
## Amendment (2026-07-29): the biometric trait cannot be implemented on Android
|
|
|
|
Decision 1 above assigns adapter implementations to this crate. While
|
|
attempting the `PayFlowHandler` → `PaymentCoordinator` migration, one of those
|
|
adapters turned out to be **impossible to write correctly**, and the reason is
|
|
worth recording rather than working around quietly.
|
|
|
|
`BiometricAuthorizer` documents a blocking contract: `authenticate` waits for
|
|
the user and returns `Ok(())` on success. `robius_fingerprinting::authenticate`
|
|
issues a JNI call that displays the prompt and returns `Ok(())` **as soon as
|
|
the prompt is on screen**. The user's answer arrives later through
|
|
`next_event()`.
|
|
|
|
An adapter therefore has two options and both are defects:
|
|
|
|
- return `Ok(())` when the prompt opens, so
|
|
`PaymentCoordinator::authorize_and_dispatch` sends money *before the user
|
|
has touched the sensor* — review defect **S2**, the fail-open biometric,
|
|
reintroduced through the type system rather than by a careless branch;
|
|
- block the calling thread, which stops the pump that delivers the answer.
|
|
|
|
`AuthorizationAttempt` (`nigig-pay-domain::authorization`) replaces the trait
|
|
for callback-driven platforms. Authorization is a state machine —
|
|
`Requested → Prompting → Granted | Denied` — advanced by inbound signals, and
|
|
only `Succeeded` grants dispatch. A grant is bound to one intent, is spent on
|
|
use, and cannot be revived by a late or replayed event.
|
|
|
|
The trait keeps its blocking contract for genuinely synchronous authorizers
|
|
and test doubles, and now documents that Android must not use it.
|
|
|
|
`PaymentCoordinator::dispatch_with_authorization(id, &mut attempt)` is the
|
|
matching entry point. It does not prompt, does not wait, and does not consult
|
|
the injected `BiometricAuthorizer` at all — a test asserts that a granted
|
|
attempt dispatches even when the injected authorizer reports no hardware and
|
|
would error. The grant must name the intent being dispatched, must be
|
|
unspent, and is consumed on use.
|
|
|
|
`authorize_and_dispatch` remains for synchronous authorizers. Both paths
|
|
converge on the same `transition_and_dispatch`, so the duplicate-dispatch
|
|
budget and the state machine apply identically.
|
|
|
|
## Consequences
|
|
|
|
- Out-of-order, duplicate and foreign callbacks are refused by construction
|
|
rather than by reviewer vigilance.
|
|
- No platform error can authorise an automatic retry once the request may have
|
|
reached the provider.
|
|
- A forged SMS with a plausible code and a spoofed sender still cannot settle
|
|
a payment: the strict reader's output type has no settled state to reach.
|
|
- A release build cannot contain a fake gateway.
|
|
- The web payment claim is gone, and its absence is tested.
|
|
- **The AccessibilityService rail remains a business decision, explicitly
|
|
unresolved, and is not enabled by default.**
|