# Nigig Pay — implementation audit against the consolidated review - **Date:** 2026-07-29 - **Reviewed against:** `REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md` - **Method:** every claim below was checked against the code, not against the tranche notes in `PAY_CAD_IMPLEMENTATION_STATUS.md`. Where the two disagree, the code wins and the disagreement is recorded. ## Summary | Phase | Scope | State | |---|---|---| | 0 | Immediate containment | **Done**, with 0.1 deliberately re-pointed (below) | | 1 | Build, ownership, quality gates | **Done** for the owned crates | | 2 | Domain layer | **Done** | | 3 | Secure repository | **Done** except Android Keystore provisioning | | 4 | No blocking I/O in `draw_walk` | **Done and verified** | | 5 | Harden platform gateways | **Done** except 5.2, which is not an engineering task | | 6 | Truthful UX | **Substantially done**; session ownership outstanding | | 7 | Test strategy | **Done** except an adversarial corpus for the UI layer | | 8 | Verification and release discipline | **Partly**; simulator and property tests exist, pilot discipline does not | **Test counts:** domain 137, platform 64, storage 41, pay UI 61, M-Pesa observation 29. All pass under the isolated runners. --- ## Phase 0 — containment | Item | State | Evidence | |---|---|---| | 0.1 dispatch behind a compile-time flag | **Implemented, default re-pointed** | `demo` exists at every level and CI exercises both directions. It is now **on** by default because USSD automation is the product's primary function; a release build uses `--no-default-features`. Containment moved to packaging, not removed. | | 0.2 no fail-open biometric | **Done** | `AuthorizationAttempt`: only an explicit success grants dispatch. A displayed prompt does not. | | 0.3 remove PIN capture | **Done for packaging builds** | `form_pin` is `#[cfg(feature = "demo")]`; `tools/check-no-pin-capture.sh` proves by compile probe that it does not exist under `--no-default-features`. A default build captures a PIN because it dispatches. | | 0.4 SMS/USSD output is evidence, not verification | **Done** | `ObservedEvidence`; no API returns a settled state from a parse. | | 0.5 no fake desktop success | **Done** | `MockGateway` is `cfg`-gated and `compile_error!`s in release without an explicit opt-in. | | 0.6 stop persisting raw SMS | **Done** | Only a digest is retained; `store.rs` omits `raw_message` from disk. | | 0.7 pinned toolchain / recovery branch | **Done** | `rust-toolchain.toml`, `--locked` builds. | | 0.8 threat model and risk register | **Done** | `THREAT_MODEL.md`, `PAYMENT_RISK_REGISTER.md`. | ### On 0.1, explicitly The review asked for a compile-time flag so that a *production* build cannot dispatch. That was implemented as default-off, which had two consequences the review did not intend: 1. ordinary development and device testing could not use the app's main feature without knowing an undocumented flag; 2. the flag was **unreachable from `pageflipnav`**, the crate that builds the APK, until commit `9f0e133`. No APK could enable automation by any means. The flag now defaults on and a packaging build opts out. The safety property the review wanted — *a shipped build cannot dispatch* — is still available and CI-enforced; it is now a release decision rather than a development obstacle. --- ## Phase 1 — build, ownership, quality gates Done for `nigig-pay-domain`, `nigig-pay-storage`, `nigig-pay-platform`: checked-in lockfiles, `--locked` builds, `cargo fmt --check`, `clippy -D warnings`, `cargo-deny`, and a Makepad-import boundary check. Ownership is recorded in ADR 0002 and machine-checked. **Not done:** the same gates do not apply to `nigig-pay-ui`, which is linted only by `cargo test --lib`. Residual counts there: **10 `unwrap()`**, **6 `let _ =`**, 0 `dbg!`/`println!`. --- ## Phase 2 — domain layer Complete. `Money` (exact minor units), the validated state machine, `PaymentCoordinator`, correlated operations, separated outcomes, no automatic retry on ambiguity, and one `compute_adjusted_amount`. --- ## Phase 3 — secure repository SQLite with migrations, single writer, `Result` on every operation, retention/erasure policy, integrity checks, and online backup/restore. SQLCipher works and is tested under a feature. **Not done:** 3.1 key provisioning. The Android Keystore `DatabaseKeyProvider` does not exist, so encryption at rest is available but unkeyed by a real hierarchy. --- ## Phase 4 — blocking I/O Done and **verified by compilation**, which earlier tranches could not do. `reload()` is out of `draw_walk`, the SMS scan is on a real timer, summaries are computed on the event path, and the storage worker owns the only writer. --- ## Phase 5 — platform gateways `nigig-pay-platform` owns the seam: single-flight correlation, fail-closed classification driven by dispatch progress, typed correlated events, a strict SMS reader, a mock that cannot ship, and an honest web refusal. `#![forbid(unsafe_code)]`; `UssdGateway` is `!Send`/`!Sync`. **Not done — 5.2, the AccessibilityService legal review.** This is a business decision about Google Play policy exposure, recorded as blocking in ADR 0007. No amount of engineering closes it. --- ## Phase 6 — truthful UX `PaymentPresentation` makes the exit criterion a type: no `Success` variant is reachable from untrusted evidence, and no `Failed` variant from a missing SMS. `ConfirmationSummary` makes every required term mandatory and binds consent to a digest of the exact terms shown. `PaymentBatch` makes `3/2` progress unrepresentable. **Not done:** the `thread_local!` `PayFlowHandler` still owns the USSD session lifecycle, the shadow pending-store writes, and the bulk queue. Moving those changes who cancels on teardown and who observes an out-of-order callback, which needs ADR 0007's device matrix. **Not done:** 6.6 bulk controls — per-item review, limits, pause/resume and audit export. --- ## Phase 7 — tests 7.1–7.2, 7.4, 7.6–7.8 done. 7.3 done: UI tests run in CI. 7.5 done for the platform SMS reader — spoofing, forged codes, replay, unicode. **Not done:** an adversarial corpus for the UI layer specifically, and no consent-safe real-world SMS corpus exists in the repository. --- ## Phase 8 — verification and operations 8.1 deterministic simulator and 8.2 property tests exist in the domain crate. **Not done:** 8.3 fuzzing, 8.4 SBOM and formal security review, 8.5 operational telemetry, 8.6 pilot discipline. These are programme activities rather than code changes. --- ## Priority defects — current state | Priority | Defect | State | |---|---|---| | P0 | Biometric error dispatches anyway (S2) | Fixed — `AuthorizationAttempt` | | P0 | PIN in plaintext prefs (S1) | Fixed — Java scrub; field absent in packaging builds | | P0 | SMS marks payment verified (B2) | Fixed — evidence only | | P0 | Auto-retry of ambiguous USSD (B3) | Fixed — dispatch budget + spent grants | | P0 | `f64` money ledger (B6) | **Partly** — arithmetic and boundary validated; stored representation still `f64` | | P0 | Plaintext records, swallowed writes (S3/B16) | Fixed — SQLite, explicit errors | | P0 | Duplicate parser/store/flow copies (A5) | Fixed — ADR 0002, CI-enforced | | P0 | Classification written, never read (B1) | Fixed | | P1 | `thread_local` coordinator (A2) | **Open** — the remaining Phase 6 item | | P1 | 5-minute no-SMS → Failed (U4) | Fixed — `PendingConfirmation` | | P1 | Hard-coded 2024 fee table (U9) | **Open** — still a static table | | P1 | Fee lookup silently free (B5) | Fixed at every call site found | | P1 | Bulk USSD flow (U7) | Accounting fixed; controls not built | | P1 | Disk I/O in `draw_walk` (P1) | Fixed and verified | | P1 | Broken month navigation (B7) | **Fixed in this audit** — was live; see below | | P1 | Lossy PSV escaping (B4) | Fixed | | P2 | Custom JSON parser (B10) | **Unverified** | | P2 | Header forgery, cert pinning (S7/S8) | **Open** — exchange APIs untouched | | P3 | Matrix session encryption (S11) | **Open** | --- ## B7, found live during this audit The tranche notes had never examined month navigation. It was still broken in **both** copies of the transactions widget: ```rust MpesaFilterMode::Month => current + Duration::days(31 * delta), MpesaFilterMode::Year => current + Duration::days(366 * delta), ``` Reproduced before changing anything: ``` 2026-03-31 -1 month => 2026-02-28 (correct by luck) 2025-12-28 -1 month => 2025-11-27 (drifted a day) 2026-03-30 -1 month => 2026-02-27 (drifts; repeated steps skip a month) 2027-06-15 +1 year => 2028-06-15 (366d is wrong on a non-leap year) ``` Now uses `checked_add_months`/`checked_sub_months`, which clamp to the end of the target month, and `checked_add_signed` on the day path so an absurd delta reports overflow instead of panicking. An unrepresentable date leaves the view where it was rather than jumping somewhere arbitrary. This is a good illustration of why this audit reads code rather than notes: the item was neither claimed done nor flagged open, it was simply never looked at. ## What I would prioritise next 1. **`nigig-map` does not compile** (12 errors, `no field center_lat`). This blocks a full `pageflipnav` build today and is unrelated to Pay. Nothing else matters until an APK can be produced. 2. **5.2** — the Play-policy decision. It determines whether the USSD rail has a future at all, and several open items (6.6 bulk, the coordinator migration) are only worth doing if it does. 3. **The `thread_local` migration**, once a device is available to exercise the matrix. 4. **U9 versioned fee policy** — the 2024 table will go stale silently. 5. **Quality gates for `nigig-pay-ui`** — 10 `unwrap()` in a payment UI is the same defect class the domain crates already ratchet against.