Commit graph

6 commits

Author SHA1 Message Date
7fdf436510 test(pay): Phase 5 lifecycle matrix as domain tests (R2.3)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Successful in 2m36s
Payment domain, storage, platform and UI / payment-ui-tests (push) Successful in 3m17s
PDF engine / engine (push) Successful in 46s
PDF engine / makepad-integration (push) Successful in 3m23s
PDF engine / fuzz (push) Has been skipped
The exit criterion names five scenarios: permission denial, cancellation,
backgrounding, app restart, out-of-order callbacks.

Four of the five are state questions, not hardware questions. A device adds
confidence that Android really emits a given callback sequence; it cannot
tell you how the domain reacts, because the state machine decides that. So
the matrix runs against the real coordinator on every commit instead of when
a phone is free, and a regression names the invariant it broke.

13 tests in crates/nigig-pay-domain/tests/lifecycle_matrix.rs, including the
cases that only exist as races: a success arriving after a cancellation;
backgrounding before a grant (must refuse) versus after one (must be
preserved — the user did authorise); restart before dispatch versus after; a
foreign grant; a replayed grant. Plus a clean-path test so the matrix cannot
pass by refusing everything.

## A coverage hole the matrix found

a_restart_after_dispatch_cannot_redispatch passed with the duplicate-dispatch
budget removed. The state machine refuses Submitted -> Dispatching first, so
the budget was never reached. That is good defence in depth and bad
coverage — nothing proved the budget still worked.

the_dispatch_budget_survives_a_state_machine_walk_back forces the intent back
to Dispatching, exactly as a faulty recovery path would, leaving the budget
as the only guard. It fails when the budget is removed.

The forcing hook is behind a `test-hooks` feature, not #[cfg(test)]: an
integration test is a separate crate and does not see cfg(test), so the
method was simply missing. The isolated runner enables it explicitly,
otherwise that test is silently filtered out and proves nothing.

## Verified by injection

  authorization gate removed  -> 7 of 13 fail
  dispatch budget removed     -> 1 fails (the new one)

## What still needs hardware

That Android actually produces these sequences: permission dialogs,
process-death timing, callback ordering under memory pressure. This file
asserts the response is correct for each sequence; a device confirms the
sequences are the real ones. Different claims, both needed. Tracked as R2.3b.

## Validation

  domain 148 unit + 13 matrix, fmt, clippy -D warnings, bench    pass
  storage 46 / platform 64 / mpesa 29 / pay-ui 78                pass
  clippy -p nigig-pay-ui --no-deps -D warnings                   0 errors
2026-08-02 09:47:52 +00:00
d567978119 feat(pay): key rotation for the encrypted ledger (R2.2)
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Failing after 2m19s
Payment domain, storage, platform and UI / payment-ui-tests (push) Has been cancelled
Examined R2.2 the way R2.1 turned out to need, rather than assuming the
whole item was device-blocked.

Most of 3.1 was already present: DatabaseKeyProvider, open_encrypted,
wrong-key rejection distinct from corruption, keystore-unavailable failing
closed, and a test asserting no PII appears in the raw file.

## The real gap was rotation, and it is pure logic

A StaticTestKeyProvider key lives forever. An Android Keystore key does not:
KeyPermanentlyInvalidatedException is thrown after fingerprint re-enrolment,
adding or removing a screen lock, or a device restore. That is ordinary, not
exceptional. With no rotation path the only responses were "lose the ledger"
or "keep using a key that no longer exists" — and the second is not
available, because the key is gone.

Deleting the ledger is not an option either. It destroys the record of money
that may have left the account, which is the same reasoning that makes
retention.rs refuse to sweep unreconciled rows.

rotate_key uses PRAGMA rekey, which re-encrypts every page inside SQLCipher's
own transaction, then proves the new key reads the data before returning — a
rekey that reported success but left the file unreadable would otherwise only
surface on the next launch, by which time the old key may be gone.

5 tests: records survive rotation, the superseded key stops working, an empty
key is refused without damaging the file, rotation is repeatable, and the
schema version is untouched. Verified by neutering rotate_key: 3 fail.

Storage tests 41 -> 46.

## Ordering, documented at the trait

The caller persists the new key only after rotate_key returns Ok. The reverse
order leaves a stored key that does not open the file. This order leaves, at
worst, a re-keyed file whose new key was not saved — recoverable by rotating
again from the old key still in the keystore.

## What remains

The JNI call itself: KeyGenParameterSpec with user authentication required,
the AndroidKeyStore provider, and catching KeyPermanentlyInvalidatedException.
The full contract is written on DatabaseKeyProvider so it is not rediscovered
from scratch. Tracked as R2.2b. Everything except the platform call is
already exercised by the sqlcipher suite.

## Validation

  storage 46 (was 41) with --features sqlcipher                  pass
  domain 148 / platform 64 / mpesa 29 / pay-ui 78                pass
  clippy -p nigig-pay-ui --no-deps -D warnings                   0 errors
  builds: pay, mpesa, core                                       pass
  rotation injection: 3 tests fail when rotate_key is neutered   pass
2026-08-02 09:37:00 +00:00
1d3e6ab72a fix(pay): correlate USSD callbacks to the payment that asked for them
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
repo hygiene / hygiene (push) Has been cancelled
doc-engine / consumer (push) Successful in 3m56s
Payment domain, storage, platform and UI / payment-ui-tests (push) Failing after 4s
doc-engine / engine (push) Successful in 15s
nigig-map / test (push) Failing after 2s
Payment domain, storage, platform and UI / isolated-payment-tests (push) Failing after 1m58s
sms / gates (push) Successful in 3s
sms / robius-sms (push) Successful in 22s
sms / android (push) Successful in 21s
sms / nigig-sms (push) Successful in 4m6s
sms / supply-chain (push) Successful in 4s
R2.1. Review items 2.7 and 5.3.

## SessionRegistry was built in Phase 5 and never wired

The pump still read:

    while let Some(ev) = robius_ussd::next_event() {
        ... if let Some(id) = h.current.take() { ... }
    }

next_event() drains a process-wide queue and its entries carry no session
id, so every event was applied to whatever `current` happened to be.

Reproduced before changing anything: payment A is dispatched then abandoned
with events still queued; payment B starts; the pump drains A's ResultText
and SessionEnded and applies both to B. An abandoned payment settles the one
that replaced it.

## Now

- Dispatch claims the single in-flight slot. The USSD backend returns no
  session handle, so the intent id is the correlation id — enough, because
  the registry only has to tell this payment from the previous one.
- Every event is admitted against the live operation before it can touch an
  intent. Foreign and stale events are logged and dropped.
- Terminal events are de-duplicated; progress chatter still repeats freely.
  ussd_duplicate_key mirrors ProviderSignal::duplicate_key in the platform
  crate, and a test pins the two together.
- All six terminal and teardown paths retire the session id, so a late
  duplicate cannot revive a closed operation.

6 tests, including the abandoned-payment scenario by name. Verified by
removing the close call: that test goes red. CI gate asserts the pump still
admits, dispatch still claims, and at least six paths still close — matching
the method rather than a receiver literal, because rustfmt wraps the call.

## What this does not do

The pump still lives in PayFlowHandler, which still owns the pending-store
writes and the bulk queue. Moving *ownership* to PaymentCoordinator changes
who cancels on teardown and who observes an out-of-order callback, which is
what ADR 0007's device matrix exists to check. That is now tracked as R2.1b.

The correlation defect — the one that could settle the wrong payment — is
closed, and it did not need a device. I had previously filed the whole of
R2.1 as device-blocked; that was too coarse.

## Validation

  nigig-pay-ui 78 (was 72) / nigig-mpesa 20                        pass
  domain 148 / storage 41 / platform 64 / mpesa 29                 pass
  clippy -p nigig-pay-ui --no-deps -D warnings                     0 errors
  builds: pay-ui, pay, mpesa, core; default and --no-default       pass
  correlation injection: abandoned-session test fails without it   pass
  pin-capture guard                                                pass
2026-08-02 09:21:45 +00:00
5800beb552 fix(pay): close the R1 gaps against the completion standard
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Payment domain, storage, platform and UI / isolated-payment-tests (push) Failing after 2m2s
Payment domain, storage, platform and UI / payment-ui-tests (push) Failing after 3s
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.
2026-08-02 08:52:32 +00:00
a264f53eb7 feat(pay): complete phase R1 of the remaining-work plan
Some checks failed
repo hygiene / hygiene (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
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
2026-08-02 08:33:16 +00:00
2649928867 docs(pay): execution plan for the remaining review items
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Ordered by what blocks what, not by review phase number. Three gates decide
the sequence: a human Play-policy decision, an Android device, or neither.
Work needing neither is scheduled first, because it is the only work that
can be finished rather than started.

R1 (no device, no decision):
  R1.1 versioned fee policy (U9) — the only open item that shows a user a
       wrong number during normal use. The 2024 table goes stale silently.
  R1.2 clippy gate for nigig-pay-ui (Q2)
  R1.3 exchange API hardening (S7/S8/S10) — needs an option chosen
  R1.4 adversarial corpus for the UI layer

R2 (needs a device): session ownership, Keystore provisioning, the device
matrix. Each exit criterion is a device behaviour, not a compile.

R3 (needs a decision): 5.2 the AccessibilityService policy review, and 6.6
bulk controls which depend on its outcome.

R4: programme discipline — fuzzing, SBOM, telemetry, pilot.

Corrects an earlier claim of mine. I reported "10 unwrap() in the payment
UI". Checked properly: 9 are inside #[cfg(test)] and legitimate. Exactly one
is production code, at pay_flow_handler.rs:245, on the dispatch path inside
the biometric branch. It is currently unreachable but nothing enforces that,
and it panics mid-payment if it becomes reachable. The count was wrong; the
item is still real, and smaller than stated.

Also records what R1.3 actually found: the exchange client forges
origin/referer for api2.bybit.com and p2p.binance.com, which is
impersonating those exchanges' own web clients against internal endpoints,
not merely a missing pin.

B6's stored representation is deferred on purpose, with the reasoning.
2026-08-02 08:20:40 +00:00