andodeki
23fce675de
Option A, as requested. Plus an audit of the whole review against the code. ## The default flips nigig-pay-ui default = [] (leaf stays off; see below) nigig-pay default = ["demo"] nigig-mpesa default = ["demo"] pageflipnav default = ["native", "demo"] `cargo run -p pageflipnav` now drives *334#, shows the PIN field and dispatches. That is the app's primary function and it works out of the box. A release build opts out: cargo build -p pageflipnav --no-default-features --features native This was not one line. A first attempt flipped the four `default =` lines and the opt-out still leaked: the app crates depended on nigig-pay-ui with its own defaults, so `--no-default-features` on pageflipnav was silently re-enabled one level down. Verified with a compile probe rather than cargo tree, which truncates. The inner deps now carry `default-features = false`, and the probe confirms both directions: default -> demo ON, --no-default-features -> demo OFF. ADR 0007's Play-policy note is untouched. The containment requirement of review item 0.1 is not dropped — the flag exists, CI exercises both directions, and a shipped build still cannot dispatch. What changed is which way it points by default, so development and device testing are not fighting it. CI guards inverted to match: they now assert automation is on by default *and* that the packaging opt-out still works. check-no-pin-capture.sh now probes the packaging build, since the default legitimately captures a PIN. ## REVIEWS/IMPLEMENTATION_AUDIT.md Every phase checked against the code, not against the tranche notes. Where they disagreed the code won. Summary: phases 0-5 and 7 done bar 5.2 and Keystore provisioning; phase 6 substantially done with the thread_local session ownership outstanding; phase 8 partly. ## B7 found live while auditing Month navigation had never been examined. Both copies of the transactions widget still stepped months with Duration::days(31) and years with Duration::days(366). Reproduced before touching it: 2025-12-28 -1 month => 2025-11-27 (drifts 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 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. An unrepresentable date leaves the view where it was. Neither claimed done nor flagged open — simply never looked at. That is the argument for auditing code rather than notes. ## Validation domain 137 / storage 41 / platform 64 / mpesa 29 / pay-ui 61 pass cargo check: pay-ui, pay, mpesa, core (default and opt-out) pass demo-on-by-default probe, both directions pass no-PIN-capture guard against the packaging build pass Unrelated and still blocking a full APK: nigig-map fails to compile on clean HEAD (12 errors, no field center_lat on ViewportState).
9.7 KiB
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:
- ordinary development and device testing could not use the app's main feature without knowing an undocumented flag;
- the flag was unreachable from
pageflipnav, the crate that builds the APK, until commit9f0e133. 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:
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
nigig-mapdoes not compile (12 errors,no field center_lat). This blocks a fullpageflipnavbuild today and is unrelated to Pay. Nothing else matters until an APK can be produced.- 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.
- The
thread_localmigration, once a device is available to exercise the matrix. - U9 versioned fee policy — the 2024 table will go stale silently.
- Quality gates for
nigig-pay-ui— 10unwrap()in a payment UI is the same defect class the domain crates already ratchet against.