andodeki
c6e122c3fc
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
Phase 1 of NIGIG_PAY_CONSOLIDATED_REVIEW.md is now closed. 1.1 build governance: - Declare license = "MIT" on both payment crates. cargo-deny correctly reported them as unlicensed, which would block any distribution review. - Version-pin the nigig-pay-domain path dependency; a bare path dependency is a wildcard requirement. 1.2 quality gates: - Add deny.toml and a CI job running cargo-deny over both payment crates. Advisories, bans, licences and sources all pass. The config bans the makepad-* crates outright and restricts sources to crates.io. 1.3 canonical ownership (ADR 0002): - Record the domain/storage/platform/UI layering and its one-way deps. - The review's A5 "fork farm" table is stale: one copy each of parser.rs, store.rs, pending_store.rs and pay_flow_handler.rs, not three. - Fix B1: store.rs parsed category, sub_category, status and confidence from disk then overwrote them with Default::default(), losing every user categorisation on reload. Persistence also wrote display names, which are not reversible, so this adds stable storage tokens with a legacy-display fallback so existing rows still load. - Fix B4: clean/restore mapped '|' to '~' and reversed every '~', so "JOHN~DOE" loaded as "JOHN|DOE". Replaced with bijective backslash escaping covering the separator, newlines and carriage returns. - B6 (f64 money) deliberately deferred to Phase 6: it is a type change that ripples into UI consumers. These were previously recorded as untestable because nigig-core is not a workspace member. That was wrong: the three files involved need only serde, chrono, one log! macro and one app_data_dir() helper. The new tools/test-mpesa-store-clean.sh supplies those shims in a throwaway crate and runs 9 tests, two of which reproduced the defects before the fix. 1.4 boundary: enforced twice, by a CI manifest/import check and by the deny.toml ban list. 1.5 shims: three re-exports in nigig-pay/src/lib.rs had zero callers and are deleted. The remaining four carry a caller count and a named migration target so they have a deletion plan rather than an open-ended lifetime. 1.6 scope (ADR 0001): accepted that Nigig Pay is a read-only tracker and launcher, not a payment processor, until an authorised provider integration exists. This is the decision the review required before further UI work. SECURITY: a live Cloudflare API token was found committed in README.md, present since the initial commit and pushed to a public remote. Removed and recorded as R-SEC-001 in PAYMENT_RISK_REGISTER.md. Redaction does not revoke it; it remains in history and must be rotated by the owner. Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt, clippy -D warnings and cargo-deny; storage also passes --features sqlcipher; domain benches run; 9 M-Pesa store tests pass. 49 manifests parse.
105 lines
5 KiB
Markdown
105 lines
5 KiB
Markdown
# ADR 0001: Payment scope — tracker/launcher, not payment processor
|
|
|
|
- **Status:** Accepted
|
|
- **Date:** 2026-07-27
|
|
- **Review item:** 1.6 (Phase 1, `NIGIG_PAY_CONSOLIDATED_REVIEW.md`)
|
|
- **Supersedes:** nothing
|
|
- **Related:** `THREAT_MODEL.md`, `PAYMENT_RISK_REGISTER.md`
|
|
|
|
## Context
|
|
|
|
The review's central finding is that this is "a prototype with payment-shaped
|
|
code, not a safe payment product". Phase 1 requires an explicit architectural
|
|
decision about which of two products this actually is, because almost every
|
|
remaining engineering decision depends on the answer:
|
|
|
|
1. a **read-only finance tracker and launcher** that observes M-Pesa activity
|
|
and hands the user off to Safaricom's own interfaces to authorise money
|
|
movement; or
|
|
2. a **provider-authorised payment processor** that initiates transactions
|
|
itself.
|
|
|
|
Today the code attempts option 2 through a mechanism that cannot support it.
|
|
Money movement is driven by an Android `AccessibilityService` that walks the
|
|
system dialer's node tree and injects the amount, the till and the customer's
|
|
M-Pesa PIN. That approach has four disqualifying properties:
|
|
|
|
- **No provider authority.** Safaricom has not authorised this integration.
|
|
There is no Daraja contract, no merchant short code, no API credential, and
|
|
therefore no idempotency key, no server-side transaction record and no
|
|
reconciliation endpoint to query.
|
|
- **No trustworthy confirmation.** Without a provider callback the only
|
|
evidence available is a parsed SMS, whose sender ID is spoofable. The
|
|
codebase now models this correctly as `ObservedEvidence`, which by
|
|
construction cannot settle a payment.
|
|
- **Play policy exposure.** The Accessibility API allow-list does not include
|
|
"automate financial transactions". This is the exact primitive banking
|
|
trojans abuse, and it is the pattern Play Protect fingerprints.
|
|
- **Unavoidable PIN custody.** Driving the dialer requires possessing the
|
|
customer's PIN. No amount of local hardening makes an unlicensed third party
|
|
a safe custodian of a mobile money PIN.
|
|
|
|
These are contractual and regulatory facts, not code defects. No refactor
|
|
removes them.
|
|
|
|
## Decision
|
|
|
|
**Nigig Pay is a read-only M-Pesa tracker and launcher until an authorised
|
|
provider integration exists.** Concretely:
|
|
|
|
1. The default production build must not initiate a money movement. USSD
|
|
automation, auto-retry and bulk pay stay behind the non-default `demo`
|
|
feature (already enforced, review item 0.1).
|
|
2. The product never collects, stores, transmits or transits an M-Pesa PIN on
|
|
any default path. Where the user must authorise a payment, the app hands
|
|
off to Safaricom's own SIM toolkit, USSD menu or M-Pesa app, and the
|
|
customer authenticates there.
|
|
3. All transaction data displayed by the app is **observational**. The UI must
|
|
distinguish "confirmed by the provider" from "observed locally" and from
|
|
"unknown, awaiting reconciliation", and must never present the latter two
|
|
as settlement.
|
|
4. The domain layer is nonetheless built to processor standards — exact
|
|
`Money`, a validated state machine, durable intents, correlation IDs,
|
|
evidence separation, an explicit reconciliation queue. This is deliberate:
|
|
it is what makes option 2 reachable later without another rewrite, and it
|
|
is what keeps the tracker honest today.
|
|
5. Moving to option 2 requires **all** of: an executed Safaricom Daraja (or
|
|
equivalent) agreement; a server-side component holding the credential;
|
|
provider-issued idempotency and reconciliation; a security review of the
|
|
full key hierarchy; and removal of the AccessibilityService payment path.
|
|
Until every one of those exists, option 2 stays closed.
|
|
|
|
## Consequences
|
|
|
|
**Positive.**
|
|
|
|
- The product can ship. A tracker that reads SMS the user already receives,
|
|
categorises spending and deep-links to official M-Pesa flows is defensible
|
|
under Play policy and needs no provider contract.
|
|
- The severity of the outstanding P0 defects drops sharply, because the
|
|
dangerous path is not in the default build.
|
|
- The architecture built in Phases 2 and 3 is not wasted; it is the
|
|
foundation option 2 would need.
|
|
|
|
**Negative.**
|
|
|
|
- Bulk pay, one-tap send and in-app payment confirmation are not deliverable
|
|
as designed. Bulk pay in particular must stay disabled: without provider
|
|
idempotency it is a duplicate-payment generator.
|
|
- Marketing must not describe the product as a payment app, a wallet, or a way
|
|
to send money. It sends nobody's money; it observes and launches.
|
|
- Some UI work in `nigig-pay-ui` is written against the processor assumption
|
|
and will need rework under Phase 6.
|
|
|
|
**Neutral.**
|
|
|
|
- The `demo` feature remains for development and demonstrations. It is
|
|
compile-time gated, must never be enabled in a distributed artefact, and its
|
|
presence is not a licence to describe the product as a payment processor.
|
|
|
|
## Compliance
|
|
|
|
Any change that would let a default build initiate a money movement, collect a
|
|
PIN, or describe unconfirmed activity as settled contradicts this ADR and must
|
|
either be rejected or accompanied by a superseding ADR that records the
|
|
provider agreement making it lawful.
|