15 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 5800beb552 |
fix(pay): close the R1 gaps against the completion standard
You are right that my first pass at R1 fell short. It deferred an item on a judgement call, and it fixed three defects without regression tests naming them. Four gaps, all closed here. ## 1. S8 was deferred; it is now done as far as the platform allows I skipped certificate pinning as "wasted work if the endpoints get dropped". That was my call to make about effort, not an external blocker. Investigated properly: Makepad's HttpRequest exposes no pinning API. Its only TLS control is set_ignore_ssl_cert, which weakens verification. Pinning is not implementable at this layer without patching the platform crate. What *is* enforceable is the property pinning mostly buys — that a mistyped, injected or attacker-supplied URL cannot be dialled. check_transport gates every request on HTTPS plus a four-host allowlist, at all three dial sites in both copies of the client. 7 tests: lookalike hosts (api.coingecko.com.evil.example), embedded credentials (https://evil@real/), explicit ports, plain HTTP, malformed URLs, and an assertion that TLS is never disabled. Verified by disabling the allowlist: 3 tests fail. ## 2. The 13-digit phone defect had no test naming it I fixed it and moved on. It now has a regression test quoting the original duplicated branches, plus a property test that normalisation output is either empty or exactly a valid 10-digit 07/01 number — no third outcome. ## 3. The fee-policy UI wiring was untested The domain guard had 11 tests; the wiring that connects it to the pay sheet had none, so nothing proved the sheet actually consults it. Four tests now cover the shipped policy: it identifies the bundled tariff, refuses once stale, still quotes while current, and keeps "unknown band" distinct from "stale table". ## 4. The exchange client had no tests at all It does now, via the transport module above. ## A test that failed against itself tls_verification_is_never_disabled_in_this_module asserts the module never calls set_ignore_ssl_cert — and the literal in the assertion put the string in the file, so it failed on first run. The needle is now assembled at runtime. Recorded because it is exactly the kind of thing that gets "fixed" by deleting the test. ## Completion standard, now written into the plan A phase is done when: no item is deferred on a judgement call; no capability is removed to satisfy a review item; defects found while implementing are fixed in the same phase even if absent from the review; every fix carries a test that fails without it; and CI enforces it. ## Validation domain 148 / storage 41 / platform 64 / mpesa 29 pass nigig-pay-ui 72 (was 66) / nigig-mpesa 20 pass clippy -p nigig-pay-ui --no-deps -D warnings 0 errors builds: pay-ui, pay, mpesa, core pass allowlist injection: 3 tests fail when disabled pass pin-capture guard pass Pre-existing and untouched: `cargo test -p nigig-pay --lib` fails to build on clean HEAD (ClassifiedTransaction not in scope in transact.rs). Verified by stashing. The transport tests are exercised through the nigig-mpesa copy. |
|||
| a264f53eb7 |
feat(pay): complete phase R1 of the remaining-work plan
All four R1 items. Two of them uncovered defects that were not in the review, and R1.4's corpus found a live bug. ## R1.1 versioned fee policy (U9) The band table is a static "effective Jan 2024" snapshot. When Safaricom revises a tariff, nothing notices: the old number is quoted and the user authorises a total they are not charged. FeePolicy attaches provenance and a 400-day trust horizon. Past it, fee_for returns FeeError::PolicyOutOfDate rather than a number, and the sheet refuses to quote exactly as it already does for an unknown band — "no band for this amount" and "our table is old" stay distinguishable because they need different messages. Verified by disabling the check: 4 tests fail. Domain tests 137 -> 148. ## R1.2 quality gate for nigig-pay-ui (Q2) Correcting my own earlier count: 9 of the 10 unwraps were in tests. The one production case, on the dispatch path inside the biometric branch, is now a fail-closed path — no request, no prompt, no dispatch. nigig-pay-ui now denies unwrap_used/expect_used outside tests and CI runs clippy --no-deps -D warnings. Scoped with --no-deps because matrix_client and robius-ussd carry pre-existing warnings that are not this crate's to fix, and a gate that fails on someone else's code gets disabled. Turning the lint on surfaced 13 more issues, one a real defect: normalise_phone had two identical branches, and the 13-digit "254…" arm produced an 11-digit result — not a valid MSISDN, but non-empty, so it flowed on as a recipient. The duplication was hiding it. ## R1.3 exchange API (S7/S8/S10) The client forged origin/referer for api2.bybit.com and p2p.binance.com, impersonating those exchanges' own web clients against internal endpoints. Removed from both copies (nigig-pay and nigig-mpesa — item A5 again), along with the framework-identifying User-Agent. CI rejects either regrowing. Requests are still made, now honestly identified. If those endpoints reject an honest client the P2P panes fall back to their offline cache, which is the true state of the integration rather than a disguised one. Not done, deliberately: certificate pinning. Pinning an endpoint the product may drop is wasted work, and whether to keep these endpoints is a product call recorded in the plan. ## R1.4 adversarial CSV corpus (7.5) parse_csv turns an untrusted file into a payment list. Corpus covers empty input, injection-shaped fields, overflow, NUL, RTL override, full-width digits, a 5,000-row file and malformed numbers. The bar is not "parses correctly" but "never silently produces a payment nobody intended". Verified it can fail. UI tests 61 -> 66. ## Validation domain 148 / storage 41 / platform 64 / mpesa 29 / pay-ui 66 pass clippy -p nigig-pay-ui --no-deps -D warnings 0 errors builds: pay-ui, pay, mpesa, core; default and --no-default pass pin-capture guard pass fee-policy injection: 4 tests fail with the check removed pass corpus injection: catches a fabricating normaliser pass |
|||
| 23fce675de |
feat(pay): USSD automation on by default; containment moves to packaging
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). |
|||
| 9f0e133c4b |
fix(pay): the APK crate could not reach the demo flag at all
Reported: building the pageflipnav APK, the Pay sheet shows no PIN field and reports that dispatch is unavailable, with no way to enable the *334# automation. That is a real defect and it is worse than the earlier feature-forwarding gap. pageflipnav is the crate that produces the APK. It depends on nigig-pay and nigig-mpesa but declared no `demo` feature, so: grep -c demo crates/pageflipnav/Cargo.toml -> 0 The flag was unreachable from the only build that matters. Adding `--features demo` to nigig-pay does not change what the APK contains, so every instruction I gave for enabling the automation was useless to anyone building the real app. pageflipnav now forwards it: demo = ["nigig-pay/demo", "nigig-mpesa/demo"] Verified with a compile probe rather than cargo tree, which truncated its output and initially suggested the wiring had failed: a `#[cfg(feature = "demo")] compile_error!` in nigig-pay-ui fires twice under `cargo check -p pageflipnav --features demo` and zero times without it. A CI guard asserts pageflipnav keeps forwarding the flag, and the README now leads with the pageflipnav command and states plainly that building nigig-pay alone does not affect the APK. Unrelated: nigig-map fails to compile on clean HEAD (12 errors, `no field center_lat on ViewportState`), so a full pageflipnav build is currently blocked by that regardless of this change. |
|||
| 775eec4424 |
fix(pay): remove PIN capture from default builds rather than hiding it (0.3)
Asked in review: "does the latest code have the PIN input field?" It did.
## Hiding was weaker than it looked
pin_input, form_pin and the reveal toggle were all present, and the default
build hid them at runtime in on_after_new. Three problems:
1. The DSL declared the control visible and Rust hid it afterwards, so
anything re-applying the UI definition — a hot reload, a re-instantiated
sheet — brought it back. Defect B11 already names this class.
2. Hidden is not absent. The TextInput stayed in the widget tree, and a
hidden input can still be focused or filled programmatically.
3. form_pin was compiled into every build, so any path reaching it could
populate it.
Item 0.3 asks for removal, not concealment.
## The field no longer exists without `demo`
form_pin, pin_visible, the eye-toggle handler, the text-input handler, both
PIN checks, the request construction and try_build_ussd_request are all
#[cfg(feature = "demo")]. A default build has no field to write to.
The DSL now declares visible: false on the PIN input and its reveal button,
so hidden is the default state rather than a runtime correction; demo
unhides them on init. That closes the reload path in (1).
One runtime call remains as belt-and-braces: if a hot-reloaded definition
surfaces the input, the non-demo arm wipes what was typed. It has no
form_pin to clear, because there isn't one.
## Proving absence rather than asserting it
A grep for form_pin proves nothing — it passes just as happily against a
field still present behind a runtime if. tools/check-no-pin-capture.sh is a
compile probe: it references the field outside any cfg block and requires
the default build to fail with "no field `form_pin`" while demo succeeds.
The script restores the file on every exit path.
Verified both directions: passes on current code, exits 1 when the field is
re-exposed un-gated.
## Validation
cargo check -p {nigig-pay-ui,nigig-pay,nigig-mpesa,nigig-core} pass
cargo check -p {nigig-pay,nigig-mpesa} --features demo pass
nigig-pay-ui: cargo test --lib pass (61)
domain / storage / platform / mpesa harness pass
no-PIN-capture guard: verified to fail on a re-exposed field pass
## What 0.3 still leaves open
The Java-side KEY_PIN scrubbing was already done, so 0.3 is complete for the
default product. A demo build still captures a PIN by design — that path
exists for authorised device testing and is covered by the unresolved
Play-policy decision in ADR 0007. If that decision goes against the USSD
rail, the demo path and its PIN capture are deleted with it.
|
|||
| e12a3bb5c4 |
fix(pay): reword the bulk gate message too
Third report of "Payment dispatch is unavailable in this build".
That exact string is gone from source as of
|
|||
| 3e77bf52b0 |
fix(pay): make the demo gate reachable and its message honest
Reported twice as a bug: "Payment dispatch is unavailable in this build;
no M-Pesa PIN was used".
The gate itself is correct — review items 0.1/0.3 require that a default
build cannot move money. Two things around it were not.
## 1. The obvious command did not work
$ cargo check -p nigig-pay --features demo
error: the package 'nigig-pay' does not contain this feature: demo
Only nigig-pay-ui declared the feature. Neither app crate had a [features]
section at all, so the only way in was --features nigig-pay-ui/demo, which
nobody would guess. Anyone following the natural path hit a hard error,
concluded the build was broken, and had no way forward.
nigig-pay and nigig-mpesa now forward it: demo = ["nigig-pay-ui/demo"].
A CI guard asserts both crates forward it and that both compile with it,
tested against a reverted manifest to confirm it fails.
## 2. The message read like a build error
"unavailable in this build" describes the binary, not the product, and
offers no next step. It now says what the app does and what to do:
This app tracks payments — it doesn't send them. Send in the M-Pesa app
or dial *334#, and it will appear here automatically. Your PIN was not
requested or stored.
The internal log strings that say "compile with --features demo" are left
alone: those are for developers reading logs, not users reading a sheet.
## 3. No build instructions existed
Review item 1.1 asks for them and the README had none. Added: default vs
demo build, the two caveats that matter (USSD has an Android backend only,
so demo does nothing useful on desktop; and demo must not ship because of
the unresolved Play-policy risk in ADR 0007), native library setup, and
the test commands.
## Validation
domain / storage / platform / mpesa harness pass
nigig-pay-ui: cargo test --lib pass (61)
cargo check -p {nigig-pay,nigig-mpesa} --features demo pass
cargo check -p {nigig-pay,nigig-mpesa} (default) pass
demo-forwarding guard: verified to fail on a reverted manifest pass
No change to what the gate permits. Enabling demo is still a deliberate
act with the Play-policy exposure recorded in ADR 0007.
|
|||
| cbafa92269 |
fix(pay): close the fail-open biometric the coordinator migration would open
Review defect S2, plus an amendment to ADR 0007. ## Attempting the migration found a defect in the plan The stated next step was replacing PayFlowHandler with PaymentCoordinator, which means writing a BiometricAuthorizer adapter for Android. It cannot be written correctly, and why is the substance of this change. The trait contract is blocking: "authenticate must be a blocking call that waits for user action. Returns Ok(()) on success." robius_fingerprinting::authenticate does not do that. Reading through sys/android/prompt.rs: it calls the Java authenticate static method and returns Ok(()) as soon as the prompt is on screen. The user's answer arrives later through next_event(). So an adapter can either return Ok(()) when the prompt opens — and authorize_and_dispatch then sends money before the user has touched the sensor, which is defect S2 reintroduced through the type system — or block, and deadlock the thread that must pump the callback. Had the migration been done without noticing, the result would have been a correct-looking refactor that silently reopened the review's most serious security finding. ## AuthorizationAttempt Authorization is a state machine, not a function call: Requested -> Prompting -> Granted | Denied, advanced by inbound signals. One rule, enforced by the type: only an explicit success grants dispatch. - A displayed prompt does not authorise. A touched sensor does not authorise. A non-match keeps the prompt up and stays retryable. - A grant is bound to one intent, so a callback for an abandoned payment cannot authorise the current one. - A grant is spent on use, so one fingerprint cannot authorise two dispatches (B3), and a replayed success cannot re-arm it. - A late success after a cancel is ignored, not resurrecting the payment. - Backgrounding mid-prompt denies; it never silently allows. dispatch_ussd now consumes a grant before dispatching and refuses without one. auth == None means no biometric gate was configured, which is deliberately distinct from an ungranted one. Both cancel paths abandon the grant. A CI guard asserts the gate exists and was tested with the check disabled to confirm it fails. The trait keeps its blocking contract for synchronous authorizers and test doubles, and now documents that Android must not use it. ## Validation domain : 129 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 guard: verified to fail with the gate removed pass batch-counter and settlement-tick guards: still passing pass Domain tests 117 -> 129. ## Where the migration stands The blocker is no longer unknown. PaymentCoordinator needs an authorization path that does not assume a blocking authorizer, and AuthorizationAttempt is that path, built and tested. What remains is a coordinator entry point taking an already-granted attempt instead of calling biometric.authenticate() itself, then moving USSD session ownership across. That is an API change that should be designed against ADR 0007's device matrix — permission denial, cancellation, backgrounding, app restart, out-of-order callbacks — none of which can be exercised here. |
|||
| b4ad9348d3 |
fix(pay): make batch progress bounded and derived, not counted
Review items 6.6 / U7, and B3 at batch scope. ## A progress bar that could read 3/2 While working toward retiring the thread_local handler, the bulk accounting turned out to hold a defect worth fixing on its own. bulk_done was a bare usize incremented at eight independent match arms, none bounded, and bulk_current_idx added one more while an item was in flight. Nothing tied the counter to the batch contents. The USSD pump does not guarantee one terminal event per item — an Error can be followed by a SessionEnded for the same session, and each arm increments separately: one item, two terminal events -> bulk_done=2/2 BulkItemDispatched reports 3/2 <-- exceeds total That was reproduced before changing any code, so this answers a demonstrated defect rather than a suspected one. ## PaymentBatch Progress is now a function of the batch contents, not of how many code paths ran. Each item carries exactly one BatchItemOutcome and settle() refuses a second instead of counting it, which makes finished > total unrepresentable rather than merely unlikely. Every former bulk_done += 1 routes through one settle_bulk_item helper. A CI guard rejects direct increments and was tested against the reintroduced bug to confirm it fails, same as the tick guard in tranche 8. Three properties follow from putting this somewhere testable: - finished is not succeeded. confirmed counts only provider confirmations, so a batch that ended with unknown outcomes reports "Batch finished — N confirmed, M awaiting confirmation. Do not re-send" and never carries a tick. Only all-confirmed may be ticked. - A pending item is never offered for re-run. safe_to_recreate returns only NotSent and Rejected; replaying a possibly-settled item is B3. - Cancelling does not rewrite history. cancel_remaining marks only items that never got an outcome, because cancelling a batch does not recall a request the provider may already hold. A stray callback for an unknown item id is refused rather than counted — the batch-level counterpart of ADR 0007's correlation rule. ## Validation domain : 117 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 batch-counter guard: verified to fail on the reintroduced bug pass Domain tests 104 -> 117. ## On the thread_local, again Still there; this change did not remove it. What changed is that the state carrying correctness risk — the batch accounting — no longer lives in it. PayFlowHandler now delegates every outcome to PaymentBatch, so the thread-local holds queue plumbing and platform handles rather than the numbers a user is shown. That is a smaller and more honest claim than "Phase 6 wiring complete". Replacing PayFlowHandler with PaymentCoordinator moves ownership of the USSD session and the biometric prompt, and belongs in its own change with ADR 0007's device matrix, not folded into one that also touches accounting. |
|||
| a6e8f4c04e |
fix(pay): remove the last false-success labels, and fix B6 where it is real
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Has been cancelled
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Continues Phase 6 (REVIEWS/adr/0008) and addresses defect B6.
## Two labels were still claiming settlement
Tranche 7 noted the sheet's remaining status strings were composed
inline. Two of them were not untidiness, they were the Phase 6 violation
still shipping:
ObservedEvidence -> "✓ SMS received — ref: {code} …"
BulkItemObserved -> "✓ {current}/{total} sent"
A tick is a settlement claim. The first put one beside an SMS, which
proves nothing and can be forged. The second counted dispatches as
confirmations, so five unknown outcomes rendered as "✓ 5/5 sent".
FlowStage now owns the vocabulary next to the presentation types, with
one rule: a tick requires a provider confirmation. Evidence reads
"Message received (ref X) — not yet confirmed. Do not send again until
this is resolved." A finished batch reads "Batch finished — N sent,
awaiting confirmation", which is what actually happened.
Nine call sites derive their text instead of composing it. The two
remaining ✓ in that file are fingerprint-sensor feedback — a real local
event, not a settlement claim.
A CI guard backs this, and it was tested against the old code to confirm
it fails when the regression returns. A guard that cannot fail is
decoration.
## B6, scoped to where it is real
Each f64 site was measured before changing it, because "replace f64
everywhere" is easy to claim and easy to get wrong.
format_amount was checked exhaustively from 0.00 to 2000.00 against an
exact integer reference: zero mismatches. It also rounds 1234.567 and
2.675 correctly. It is not the defect and rewriting it would be churn.
Accumulation is the defect. 10,000 realistic amounts showed no visible
drift, but f64 silently discards additions past 2^53 and offers no
overflow signal at all, so a corrupt record yields a confident wrong
total with nothing to indicate it.
So the fix went to the accumulators. update_summary in nigig-mpesa and
nigig-pay — exact duplicates of each other, review item A5 — now build a
PeriodSummary from exact minor units with checked arithmetic, and render
"—" rather than a wrapped figure when overflowed is set. The conversion
from stored f64 is explicit and validated.
store.rs::totals() has no callers; it is now #[deprecated] with the
reason rather than deleted, since nigig-core has no domain dependency and
that is a separate change.
Two regression tests state it executably: f64 silently swallows 2^53 + 1
while the exact path keeps both entries, and an impossible total is
reported rather than shown.
## Validation
domain : 104 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
settlement-tick guard: verified to fail on the old code pass
Domain tests 95 -> 104.
## Not claimed complete
The thread_local PayFlowHandler still exists. The status vocabulary is now
domain-owned, which was the part carrying correctness risk, but the
widgets still do not talk to PaymentCoordinator. That remains the Phase 6
architectural item. The parser's stored f64 amount/balance/cost are
unchanged: that is a migration, not an edit, and belongs with SQLite.
|
|||
| 750d856668 |
feat(pay): Phase 6 truthful payment states, and run UI tests in CI
Phase 6 and item 7.3 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. Rationale in REVIEWS/adr/0008. ## The rule, made into a type Phase 6's exit criterion is that the UI cannot call a transaction successful without trusted confirmation, or failed without known rejection. The old code violated it structurally: the sheet set its status inline in about a dozen places, each from whatever local signal was nearest, so there was nowhere to put the rule. The worst instance is U4. A payment whose confirmation SMS had not arrived in five minutes was marked Failed, the sheet closed, and the user saw "✗ Payment failed" — with a retry button — while the money was gone. payment_view_state.rs makes the rule a type. PaymentPresentation has no Success variant reachable from untrusted evidence and no Failed variant reachable from a missing SMS. VerificationExpired, Unknown and UnknownNeedsReconciliation all present as PendingConfirmation: not success, not failure, and no retry offered. Two defence-in-depth rows: Confirmed without a provider reference reads as unknown rather than settled, and an unclassified failure is not evidence nothing was sent. 6.2: ConfirmationSummary makes every required term a mandatory field, so a confirmation missing the fee or total is not constructible. It is built from the same quote that gets dispatched. material_digest() binds consent to the exact terms shown and is re-checked on OK — if anything material moved in between, the authorisation is void and the screen is shown again. The modal previously showed only recipient and amount. 6.3: must_stay_open() keeps pending payments visible, and the status line begins with the exact required wording, asserted by a test. 6.5: evidence rows are exposed and labelled untrusted; pending payments offer receipt, problem-reporting and data-deletion actions. ## Three defects found by writing the tests 1. Bulk quotes silently under-charged. compute_bulk_costs used filter_map over the fee lookup, so a contact outside the tariff was dropped from the fee total and the user was quoted less than they would pay. 2. The batch total could overflow — .sum() panics in debug, wraps in release. A wrapped total is a quote for the wrong amount. 3. The cost preview showed unknown fees as free via unwrap_or(0). All three are B5/B6 territory. Item 3 is B5 resurfacing in the preview path after tranche 1 fixed it in the dispatch path: fixing a defect at one call site is not the same as fixing the defect. ## A pre-existing failing test, diagnosed rather than deleted money::tests::ksh_rounds_to_nearest_cent asserted format_ksh(1.005) == "1.01" and had been failing on every run — confirmed pre-existing by stashing this work and re-running clean. The expectation is impossible, not the formatter wrong: 1.005 has no binary representation, the nearest f64 is 1.00499999999999989..., so the correctly rounded result is 1.00. This is defect B6 at its smallest. The test now says so, with a companion showing Money handling it exactly. Deleting it would have hidden a live argument for finishing B6. ## 7.3 UI tests in CI The NIGIG_TEST_PAY gate was already gone; what blocked CI was the Makepad link step. tools/makepad-native-libs.sh listed the libraries needed to compile but not libasound2-dev, libpulse-dev and libssl-dev, which are needed to link a test binary — cargo check succeeds and then "unable to find library -lasound" appears much later. The helper now installs and checks them, and a payment-ui-tests job runs the UI tests plus cargo check on nigig-pay-ui, nigig-pay and nigig-mpesa. ## Validation domain : 95 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: cargo check pass Domain 75 -> 95 tests. UI 55 -> 61, with the long-standing failure fixed. Every new CI step was run locally before commit. ## Not claimed complete Phase 6 wiring is partial. PaymentViewState exists and is tested, and the confirmation path uses ConfirmationSummary, but the sheet's remaining status strings are still set inline and the thread_local PayFlowHandler still exists. The type makes that migration mechanical; it does not perform it. 6.6 bulk stays demo-gated pending ADR 0007's unresolved 5.2. |
|||
| d0f5e74435 |
feat(pay): Phase 5 platform gateway boundary, and verify Phase 4
Some checks failed
nigig-map / test (push) Has been cancelled
Payment domain, storage and platform / isolated-payment-tests (push) Has been cancelled
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
doc-engine / engine (push) Has been cancelled
doc-engine / consumer (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
Phase 5 of REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. Design and the one
item engineering cannot close are in REVIEWS/adr/0007.
## Phase 4 is now verified, not just written
Tranche 5 implemented the draw_walk fixes and said plainly that the
nigig-pay widget edits were unbuilt. That blocker was environmental:
installing the packages tools/makepad-native-libs.sh already lists makes
the UI graph compile. Both commands the status doc listed as required
now pass:
cargo check -p nigig-pay pass
cargo check -p nigig-pay-ui pass
No code changed for this; the claim is now evidence rather than assertion.
## Phase 5: new nigig-pay-platform crate
ADR 0002 reserved this crate and marked it "not yet created".
5.1 One crate owns the seam. It is the only crate in the payment stack
that may name a platform SDK. CI enforces both directions: payment crates
may not import Makepad, and domain/storage may not import jni or the
robius platform crates.
5.3 Correlation is mandatory and single-flight. SessionRegistry admits an
event as Accepted, Duplicate or Ignored; PlatformEvent cannot be built
without a CorrelationId. Closed session ids are retired permanently, so
an abandoned session's confirmation cannot settle the payment that
replaced it. There is a test named after exactly that scenario.
2.8/5.3 Progress decides retry safety, not error kind. classify_failure
takes the failure and the DispatchProgress reached before it, and
progress is the authority. The same TemporarilyUnavailable is safely
retryable before the dial and ambiguous once the menu is being driven —
the distinction the old code could not make, which is defect B3's
mechanism. A property test asserts across the whole failure space that
nothing which may have reached the provider authorises a fresh attempt.
5.4 Fakes cannot ship. MockGateway is cfg-gated, is a compile_error! in a
release build unless allow-mock-in-release is named explicitly, and
stamps every session id with MOCK-. CI asserts the release build fails.
5.5 No unsafe, no PIN. The crate is #![forbid(unsafe_code)] so the JNI
surface stays in robius-ussd. UssdGateway is !Send/!Sync by construction,
making the main-thread requirement a compile error. The adapter leaves
the pin field empty and a test asserts it.
5.6 The web claim is withdrawn. No browser API can drive USSD and a
Daraja credential must never reach a browser, so WebGateway refuses every
call and maps to Fatal — "never sent" — which owes no reconciliation.
7.5 Adversarial SMS corpus. StrictMpesaSms is the payment-boundary
reader, deliberately separate from nigig-core's permissive tracker parser
(ADR 0007 explains why this is not the duplication ADR 0002 forbids). It
requires an exact 10-char code, exact sender-ID match so MPESA-REFUNDS
and FAKE-MPESA are refused, rejects fractional shillings instead of
rounding, and caps body length. Corpus covers spoofing, forged code
shapes, out-of-range amounts, unicode and NUL injection, and replay. The
closing test asserts the honest limit: a well-crafted forgery is still
only evidence, because the output type has no settled state to reach.
## 5.2 is not done and is not closeable here
The AccessibilityService Play-policy review is a business decision. ADR
0007 records it as blocking, states the termination exposure, and names
what must happen before the rail is enabled. USSD dispatch stays behind
the default-off demo feature. If the review fails, ADR 0001's
tracker/launcher position applies and only the dispatch adapter is lost.
## Validation
platform: 56 tests, 64 with --features ussd, fmt, clippy -D warnings
(both feature sets), cargo-deny, mock-in-release guard
asserted to fail pass
domain: 75 tests --locked pass
storage: 36 + 41 tests --locked, incl. sqlcipher pass
nigig-pay, nigig-pay-ui: cargo check pass
cargo-deny reports advisories/bans/licenses/sources ok. The isolated
runner gained a `platform` target and it runs in CI on every push that
touches the crate.
|
|||
| c6e122c3fc |
feat(pay): complete Phase 1 and fix M-Pesa store defects B1/B4
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. |
|||
| 01bc055d1e |
feat(pay): complete the partially-done review phases
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
No new phase was started. This finishes every item tranches 1-2 left partial, so Phases 1, 3, 7 and 8 are complete for the two pure crates or explicitly blocked on crates that cannot be built here. Phase 1 (build governance): - Check in Cargo.lock for both payment crates and build with --locked. .gitignore excluded them, which would have made item 1.1 a false claim. - tools/test-rust-clean.sh now reads the channel from rust-toolchain.toml instead of a hardcoded default that had already drifted, and gates on fmt, clippy -D warnings, the sqlcipher feature and the benchmarks. - CI enforces lockfile presence/freshness and a Makepad-boundary check that inspects manifests and use/extern lines rather than prose (1.4). Building on the declared 1.97.1 toolchain surfaced five lints 1.85 missed; all are fixed. The two Default impls are annotated rather than derived because each encodes a security or state-machine decision. Phase 3 (secure repository): - 3.1 encryption at rest: new encryption.rs and an opt-in sqlcipher feature. PRAGMA key is applied first and verified by a forced read, so a wrong key fails as KeyRejected rather than as corruption. DatabaseKey redacts its Debug and zeroes on drop. No key is ever derived or persisted here, and there is no unencrypted fallback. A test asserts the recipient MSISDN is absent from the raw database bytes. - 3.5: preferences.rs replaces the ANDROID_DATA marker file whose existence was the value; every field fails safe. - 3.6: redact.rs. PaymentIntent's derived Debug leaked a customer MSISDN into any log line; it now masks phone, name and ids, with a regression test. StorageError no longer prints a full intent id. Phase 7: - 7.7 benchmarks over money, validation, fees, the intent lifecycle and evidence handling, using the stable harness so they run on the pinned toolchain. BASELINE.md records measured output. - 7.1/7.2/7.3 verified already satisfied; the review text is stale. Phase 8: - 8.1 simulator.rs with a scripted gateway and biometric, no clock or I/O. - 8.2 property tests over every permutation of a representative event set: no ordering dispatches twice, untrusted events never settle a payment, ambiguity never becomes failure, Confirmed is terminal, duplicate confirmations are idempotent, and replayed SMS cannot fake a conflict. Validated on rustc 1.97.1 outside the incomplete workspace graph: domain 66 tests, storage 20 tests, storage+sqlcipher 25 tests, fmt clean, clippy -D warnings clean on both crates and both feature sets, benches run. 49 manifests parse; git diff --check clean. Still unclaimed and blocked on uncompilable crates: UI wiring, Keystore key provisioning, Phase 4 draw_walk I/O, Phase 5 adapters and legal review, the Java PIN scrub, and B1/B4/B6 in nigig-core. |
|||
| 4efd7b3780 |
ci(pay): test isolated domain and storage crates
Some checks failed
Payment domain and storage / isolated-payment-tests (push) Has been cancelled
|
