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.
9.4 KiB
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:
- Uncorrelated callbacks. A result arriving from an abandoned session settled whichever payment was current.
- Error kind mistaken for safety.
TemporarilyUnavailablewas 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. - Untrusted input treated as an outcome. A scraped dialog or an SMS whose
sender field said
MPESAwas 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-coreanswers "what should the history screen show?" A message it cannot read is a missing row, so it is permissive.nigig-pay-platformanswers "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
- 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.
- Until then, USSD dispatch stays behind the default-off
demofeature, 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, soPaymentCoordinator::authorize_and_dispatchsends 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.