95 commits

Author	SHA1	Message	Date
andodeki	63ff45149a	feat(pdf): a real JPEG decoder — the old one was a stub returning black ... Completes Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. Design and merge criteria in REVIEWS/adr/0016-pdf-image-decode-surface.md. ADR 0015 refused DCTDecode at the generic filter boundary and left the image path alone, noting JPEG "is decoded on the image path". That claim did not hold: fn decode_jpeg_data(_data, _pixels, _width, _height, _components) -> Option<()> { Some(()) } Every argument discarded. It wrote nothing and returned success. The caller allocated a zero-filled buffer, passed it in, and returned it as decoded pixels. Probing a real 8x8 JPEG through ImageInfo: decode_to_rgba -> 256 bytes, first 12: [0,0,0,255, 0,0,0,255, 0,0,0,255] Pure black at full alpha. Not an error, not None - a correctly sized, entirely fabricated image. EVERY JPEG IN EVERY PDF rendered as a black rectangle and nothing reported it. The underscore-prefixed parameters are the tell: the signature was written to silence the unused warnings that would otherwise have announced the stub. image.rs was at 14.2% line coverage, the lowest in the crate. Replaced with a real baseline decoder in pdf-graphics/src/jpeg.rs: huffman, dequantisation, IDCT, chroma upsampling, YCbCr/YCCK conversion including the Adobe APP14 transform flag. No new dependency - adding `image` or `jpeg-decoder` would pull a tree into a crate that has one, on a target the team is already fighting to cross-compile. Progressive JPEG is refused BY NAME rather than approximated; a partial implementation would reproduce exactly the defect being fixed. decode_to_rgba's Option is why the stub survived - "could not decode" and "decoded to nothing" were the same value. The decoder returns a typed JpegError so a caller learns why an image is missing. Also in this tranche, from the same plan bullets: - ImageInfo::downsample, integer-factor box filter. Refuses factor 0, and refuses data that is not raw samples rather than averaging compressed bytes as though they were pixels. - Round-trip tests for encode_flate and encode_ascii_hex over adversarial inputs: empty, single byte, all-zero, all-0xFF, random binary. THE IDCT TOOK THREE ATTEMPTS AND THE FAILURES WERE INFORMATIVE The first version, adapted from a hand-tuned integer kernel, decoded greyscale exactly (128 -> 128) while colour came out a UNIFORM 64 levels off. A constant offset across every channel is a scaling-factor mistake, not a coefficient one - guessing at coefficients would never have found it. Two rounds of guess-and-check made it worse. The fix was to stop guessing: derive ground truth from the float reference in T.81 A.3.3, then transcribe the separable form directly with a documented fixed-point scale. The cosine table is a const fn so it cannot drift from the formula beside it, and tests assert against the reference rather than our output. 4 corpus fixtures with real JPEGs (Pillow at generate time only; the .pdf files are committed so CI never needs it), 16 acceptance tests asserting PIXEL VALUES rather than buffer lengths - a length assertion would have passed against the stub. Mutation-checked: reinstating the zero buffer fails four tests. Coverage on image.rs 14.2% -> 32.9%, new jpeg.rs 82.8%, crate 83.65% -> 84.22%. TEST_TARGET=pdf 651 -> 680, TEST_TARGET=pdf-ui 696 -> 725. rustfmt and clippy -D warnings clean.	2026-08-16 18:33:56 +00:00
andodeki	fb95b25a67	fix(pdf): LZW was broken outright; refuse image codecs instead of faking them ... Phase 3 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, lossless half. Design and merge criteria in REVIEWS/adr/0015-pdf-filters-and-codecs.md. LZW DID NOT WORK Fed the worked example from PDF 32000-1 section 7.4.4.2: LZW default : Err("LZW previous code out of range") decode_lzw seeded a 256-entry dictionary but set next_code = 258, because 256 and 257 are the clear and EOI codes. New entries were appended with table.push, landing at index 256 - so the counter and the real index were permanently two apart and every dictionary reference resolved to the wrong entry. Any PDF using LZW was affected, which is a whole class of older files. Also in the same area: - /EarlyChange was ignored. It selects when the code width grows; a file setting 0 decoded to GARBAGE rather than failing, which is worse. - Predictors were applied to Flate only, though /Predictor is equally legal on LZWDecode. TWO MORE BUGS FOUND WHILE IMPLEMENTING decode_stream read /Filter as a single NAME and fell through to "unsupported filter" for an array. The document layer calls decode_stream, so every chained stream in every document failed to decode - including the common [/ASCII85Decode /FlateDecode]. It now delegates to decode_stream_with_params, leaving one decoding path. decode_flate_with_predictor inflated its own input, so calling it from a chain decompressed already-decompressed bytes. Split into apply_predictor, which works on decoded data. IMAGE CODECS: REFUSED, NOT FAKED DCTDecode and JPXDecode previously returned their COMPRESSED bytes as though decoded: "DCTDecode" | "JPXDecode" | "Crypt" => data, A caller received a Vec<u8> that looked like image data, was not, and produced garbage pixels rather than an error. CCITTFaxDecode, JBIG2Decode, JPXDecode and DCTDecode now return a typed error naming the filter. image.rs still sniffs and decodes JPEG on the image path, so that route is unaffected; what stops is the generic filter claiming a success it did not achieve. /Crypt stays a pass-through, correctly - decryption already ran. Not implementing CCITT/JBIG2/JPX is a decision, not an omission: JBIG2's CVE record is why browsers sandbox it, and JPX via openjpeg would add a C dependency that breaks the Android cross-compile the team is already fighting. CCITT is the tractable one and is the recommended next step. 4 corpus fixtures, 14 acceptance tests. Mutation-checked - and one check initially misled me: removing the reserved-slot seeding did not fail the tests, because the clear-code branch re-seeds independently and every real LZW stream opens with a clear code. Removing both fails all three LZW tests. Recorded in the ADR. One pre-existing defect deliberately left: the PNG predictors do not consume the per-row filter-type byte. Fixing it risks every Flate-with-predictor document in the corpus and is not what this ADR set out to do, so it is documented rather than quietly half-fixed. TEST_TARGET=pdf 637 -> 651, TEST_TARGET=pdf-ui 682 -> 696. rustfmt and clippy -D warnings clean.	2026-08-16 18:04:07 +00:00
nigig-ci	0c14f8d848	docs(email): correct dead commit SHAs; record the SMS parity mechanism ... Three fixes to the plan, one of them a real defect in the document. 1. Every commit SHA it cited was dead. I wrote them before the final rebase, which rewrote them, so the progress log pointed at eleven references that `git cat-file -e` cannot resolve. A plan that cites nonexistent commits is worse than one that cites none. Remapped: 28d0608 -> 5a5b817 1ea9ad6 -> b91f97b 50760e0 -> 18bbb7b 169563f -> b701ce5 2. Documented HOW the SMS similarity is achieved, since "like the SMS list" is the requirement and prose does not prove it. Added a table naming the six shared components both features now consume from nigig-uikit/src/shared/conversation/ -- the preview row, its action, its props, the stack-navigation view, the message bubbles and the bottom-nav actions. Parity is structural, not cosmetic. Also recorded the consequence: those widgets are now load-bearing for two features, so an email change can regress SMS. The shared kit has no tests of its own. New item E6. And the one intentional divergence: email rows key on a normalised (lowercased) sender address, because Alerts@Bank.co.ke and alerts@bank.co.ke are one sender. 3. Updated item 0.7. The makepad `maps` break is fixed by ce0eaae, but that commit added `i_tree = "1.0.0"` to crates/apps/map and crates.io publishes only up to 0.19.0, so the workspace still does not resolve. Verified pre-existing by stashing all email changes and reproducing on a pristine tree. This remains the highest-priority blocker: while it holds, no crate in the repo can be verified on a runner.	2026-08-16 17:44:59 +00:00
andodeki	6a18886185	feat(pdf): Type 3 fonts and streaming interpretation — Phase 2 complete ... The last two items of NIGIG_PDF_FEATURE_PARITY_PLAN.md Phase 2. Design and merge criteria in REVIEWS/adr/0014-pdf-type3-fonts-and-streaming.md. TYPE 3 FONTS DREW NOTHING A Type 3 font's glyphs are not outlines - they are content streams, listed in /CharProcs and mapped to text space by /FontMatrix. Probing a document with one: fonts on page: ["T3"] T3: subtype=Type3 base=Unknown -> are the glyph procedures reachable? no CharProcs field exists -> is /FontMatrix exposed? no field exists The font was detected and then nothing could be done with it. /CharProcs and /FontMatrix appeared nowhere in the crate, so the procedures were unreachable and the text was silently invisible - a page that renders, reports no error, and is missing content. New pdf-document/src/type3.rs parses /FontMatrix, /CharProcs, /Differences, /Widths, /FontBBox and the font's own /Resources, and resolves a character code to its glyph procedure's decoded bytes. /FontMatrix is applied as written rather than assumed to be the common 0.001 scale - Type 3 fonts routinely use other matrices, which is the point of the entry. A missing /CharProcs entry is a typed error naming the glyph, not a blank. A THIRD BUG, FOUND WHILE WIRING d0/d1 The interpreter parsed both operators and discarded them: PdfOp::Type3Width(_wx, _wy) => {} PdfOp::Type3BBox(_x1, _y1, _x2, _y2) => {} They are how a Type 3 glyph declares its advance, so even a renderer that could draw the glyphs would stack them all at one point. Wiring them to the device exposed that `d1` takes SIX operands - wx wy llx lly urx ury - and the parser read four, so the "bounding box" was really the advance and the advance was lost entirely. Now `Type3BBox { wx, wy, bbox }`, reading all six. STREAMING INTERPRETATION parse_content_stream materialised every operator into a Vec before interpreting any of them: peak memory proportional to the whole content stream, on a stream walked once and discarded. Adds ContentStreamIter and interpret_streaming, with parse_content_stream reimplemented on top of the iterator so there is ONE tokeniser rather than two that can drift. Equivalence is proven, not asserted: a test compares both paths across every corpus fixture, and a streaming_interpreter fuzz target compares them over arbitrary bytes, which is where a divergence would actually hide. 4 corpus fixtures, 13 acceptance tests, 9 unit tests. Mutation-checked: reverting d0 to a no-op fails glyph_advances_reach_the_device. Phase 2 is now complete; the plan is updated with an item-by-item audit. Several entries were already done (inline images, Do, text state, shading); the plan's "biggest gap" was xref streams, closed in ADR 0013. TEST_TARGET=pdf 615 -> 637, TEST_TARGET=pdf-ui 660 -> 682. rustfmt and clippy -D warnings clean.	2026-08-16 17:28:44 +00:00
nigig-ci	b701ce5eeb	docs(email): assessment and live execution plan ... Lands in REVIEWS/ rather than the repo root, where 35 markdown files already compete for attention. Records the audit (architecture, performance, bugs, design, security, code quality) with each finding tied to evidence that was executed, not inferred, and a progress log that marks what has shipped. Two things worth reading even if you skip the rest: - I got the port-465 TLS finding WRONG on the first pass and wrote it up as critical credential exposure. Checking lettre 0.11.23's source showed relay() is implemented with the same three calls and TlsParameters::new already sets accept_invalid_certs: false and a TLS 1.2 floor. Downgraded to Medium and the error is recorded rather than quietly removed, because a document like this is worthless if you cannot tell which claims survived scrutiny. - C1 is the single blocking decision: IMAP on device vs a server-side proxy. The inbox list, thread reader and grouping are done and work; what they display is sample data until that is answered. The plan lays out the tradeoff and does not pretend it is a technical call. Also notes that origin/main does not currently resolve — the makepad bump in 86c9595 dropped the `maps` feature pageflipnav declares — which is now item 0.7 and blocks CI verification for every crate, not just this one.	2026-08-16 17:27:35 +00:00
andodeki	2faadb777f	feat(pdf): read xref streams and object streams (PDF 1.5+) ... Phase 2 of NIGIG_PDF_FEATURE_PARITY_PLAN.md, the item it calls "the biggest parse-side gap". Design and merge criteria in REVIEWS/adr/0013-pdf-xref-streams.md. The parser could not open a PDF 1.5 file. Not render it wrong - not open it: PARSE FAILED: PDF error at byte 382: expected xref keyword XRefTable::parse_section required the literal bytes `xref` at the startxref offset. A PDF 1.5+ file has an indirect object there instead - the xref stream - so the parse aborted and the entire document was unreadable. Every feature built on top of the parser (encryption, signatures, forms, structure tree, transparency) was unreachable on any file produced in the last twenty years. ObjStm, XRefStm and /Type /XRef appeared nowhere in the crate. Implemented on the read side: - Xref streams: the packed binary table, /W field widths, /Index sparse subsections, and types 0/1/2. A zero-width /W column means "use the default" (type 1) - missing that rule yields a table of all-free entries and an apparently empty document rather than an error. - Object streams: type-2 entries resolve through /ObjStm, reading the header pairs and /First. The xref's index is used but verified against the object number it claims to be, because a wrong-but-in-range index would silently return a different object. - Hybrid files: a traditional table plus /XRefStm. Both are read, with the traditional table winning on conflict, which is the point of the layout. Bounds and refusals rather than silent degradation: /W widths are clamped and every field read is checked against the decoded buffer; a truncated table is flagged, not padded with free entries; an object claiming to live inside itself is refused; a /Type that is not /XRef is named in the error. Scope note: the writer is untouched. ADR 0003 keeps appending a traditional xref section, which remains correct - the appended trailer carries /Prev to the stream, so the chain stays readable by us and by conforming readers. Also verified against the rest of Phase 2: inline images, XObject Do, shading, and the full text state (Tc/Tw/TL/Tz/Ts/Td/TD/Tm/Tf) are already implemented and tested. Type 3 fonts and the streaming interpreter remain genuine gaps, but each degrades one feature rather than the whole file. 6 corpus fixtures, 9 acceptance tests asserting real page content rather than a successful parse, and a parse_xref_stream fuzz target because the table is attacker-controlled binary. Mutation-checked: restoring the old error fails 5 of the 9. TEST_TARGET=pdf 606 -> 615, TEST_TARGET=pdf-ui 651 -> 660. rustfmt and clippy -D warnings clean.	2026-08-16 17:10:42 +00:00
andodeki	bd01604e65	docs(pdf): Phase 1 status, and stop pdf-ui failing for an environmental reason ... Phase 1 of NIGIG_PDF_FEATURE_PARITY_PLAN.md. Findings verified by running them, written up in REVIEWS/PDF_PARITY_PHASE1_STATUS.md. Three of the five exit criteria are met: the workspace is green on the new rev (pdf 606 passing), both pins are already at 5efe6e24c, and the upstream baseline is documented - libs/pdf_parse is 4,575 lines with no save/write path at all, against nigig-pdf's 25,845 lines with writing, encryption, signatures, structure tree and transparency. nigig-pdf supersedes both libs/pdf_parse and widgets/src/pdf_view.rs; nothing in either is a capability we lack. The remaining two criteria need fork work this repo cannot do. WHY THE UI SUITE CANNOT PASS YET The six #[ignore] markers were removed from pdf-makepad/tests/ui.rs and the docs now claim the suite runs without a Studio hub. The markers went but the tests did not start passing - TEST_TARGET=pdf-ui was simply red. Three layers, each found by fixing the one in front of it: 1. studio/hub/src/build_manager.rs:398 spawns the build with `sh -lc`. The -l makes it a LOGIN shell, which discards the inherited PATH and rebuilds it from /etc/profile, where ~/.cargo/bin does not appear. cargo is not found and the child exits 127 in 0.4s. This breaks any rustup-based CI, not just this sandbox. `sh -c`, or resolving cargo through the CARGO env var, would fix it. 2. Past that the build runs (88s) and the failure becomes 101. libs/makepad_test sets MAKEPAD=headless for the child, but platform/src/os/linux/windowing_backend.rs only knows X11 and Wayland - there is no headless backend and the env var is not consulted. The app selects X11, finds no display, and segfaults (139). This is the real Phase 1 fork task: "terminal/standalone mode" needs a backend, not just an env var the harness sets. 3. Under xvfb-run the app starts properly and OpenGL initialises, so the binary is fine - but the hub spawns its child outside that display. The markers are restored, with a reason pointing at the status document. A red suite everyone knows to disregard stops reporting the next real regression, which is strictly worse than an explicit skip. Also fixes a latent build break this exposed: the fork's app_main! macro expands to #[cfg(native_activity)], a cfg this crate never declares, which is a hard error under -D warnings. Declared as expected-but-unset via [lints.rust] check-cfg rather than silencing unexpected_cfgs wholesale, which would also hide our own typos. One genuine improvement on this rev: pdf-makepad now builds in release inside the workspace. That was previously blocked by a Makepad os::linux feature-unification bug. TEST_TARGET=pdf 606 passing, TEST_TARGET=pdf-ui 651 passing + 6 ignored.	2026-08-16 16:48:02 +00:00
andodeki	7fdf436510	test(pay): Phase 5 lifecycle matrix as domain tests (R2.3) ... 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
andodeki	d567978119	feat(pay): key rotation for the encrypted ledger (R2.2) ... 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
andodeki	1d3e6ab72a	fix(pay): correlate USSD callbacks to the payment that asked for them ... 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
andodeki	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.	2026-08-02 08:52:32 +00:00
andodeki	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	2026-08-02 08:33:16 +00:00
andodeki	2649928867	docs(pay): execution plan for the remaining review items ... 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
andodeki	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).	2026-08-02 08:06:43 +00:00
andodeki	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.	2026-08-02 07:31:44 +00:00
andodeki	b720a166ec	feat(pdf): AcroForm actions and validation; JavaScript refused, not run ... Phase 8 #8 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("AcroForm full support"). Design and merge criteria in REVIEWS/adr/0012-pdf-acroform-full.md. Form editing already worked. What was missing is everything that makes a real form behave like one. Probing an invoice-shaped document: "qty" required=true /AA present? false "price" required=false /AA present? true "total" required=false /AA present? true -> is any /AA action exposed? no accessor exists -> is /CO exposed? no accessor exists The /AA dictionaries sat in the raw field dictionary, reachable only by a caller who knew to go digging. Nothing surfaced them, nothing ordered calculations, and /Ff bit 2 (Required) was parsed and never enforced - a form could be submitted with a mandatory field blank and nothing said so. THE JAVASCRIPT DECISION The review names JavaScript actions as a prerequisite. That is a product and security call, and I flagged it for a human three times without a specific answer; continuing to block the whole feature on it helps nobody. This takes the reversible option and documents it loudly enough to overrule: JavaScript is parsed, exposed, and NEVER EXECUTED. Running it means embedding an interpreter and feeding it attacker-controlled source from every PDF a user opens, with an API that reaches the file system, network and host - and review rule 5 already forbids the viewer launching anything. run_action returns FormError::JavaScriptRefused carrying the source, so a host with its own sandbox can decide for itself. A later ADR turns a refusal into an execution; nothing has to be un-built. Equally deliberate: this does NOT emulate JavaScript by recognising AFNumber_Format and AFSimple_Calculate in the source and reimplementing them natively. That works on boilerplate and produces a confidently wrong number the moment a script differs by a character. WHAT IS IMPLEMENTED - /AA parsed into typed actions across all 14 triggers, on fields and widgets, with indirect action dictionaries resolved. - /CO calculation order exposed in document order. - Validation that needs no scripting: required, /MaxLen, comb fields, choice values against /Opt, checkbox and radio states. - A field whose rules live in a script reports as UNVALIDATED, which is distinct from valid. Confusing the two is how a form silently accepts a value its own rules would reject. - /SubmitForm parsed into URL, flags and fields and returned as data. This crate opens no sockets. Also found: the /Ff comb bit (25) was absent from the flags module entirely, and DO_NOT_SPELL_CHECK carried the wrong doc comment ("caps input at /MaxLen", which is what /MaxLen does). 4 corpus fixtures, 13 acceptance tests, 20 unit tests. Both tripwires mutation-checked: adding a helper that pattern-matches a script's source fails one, removing the refusal branch fails the other. TEST_TARGET=pdf 575 -> 607, TEST_TARGET=pdf-ui 619 -> 651. rustfmt and clippy -D warnings clean.	2026-07-31 22:41:50 +00:00
andodeki	a994213e8b	feat(pdf): tagged structure tree, and the BMC bug that turned red green ... Phase 8 #7 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("PDF/UA structure tree"). Design and merge criteria in REVIEWS/adr/0011-pdf-structure-tree.md. Investigating the accessibility gap surfaced four defects in the marked-content operators the structure tree depends on. The first is not an accessibility problem at all. 1. BMC never parsed, and corrupted the colour of everything after it. The dispatcher matched b'B'+b'M' only when the third byte was a delimiter; BMC's third byte is 'C', so the arm never fired. Because the operator was never recognised it never CLEARED ITS OPERAND, and the leftover /Tag shifted the operands of whatever came next: 1 0 0 rg -> RgbFill(1.0, 0.0, 0.0) red /Span BMC 1 0 0 rg -> RgbFill(0.0, 1.0, 0.0) green Tagged documents are precisely the ones containing BMC, so the documents that tried hardest to be accessible rendered wrong colours. This is the fifth instance of the operator-shadowing family already fixed for cm, rg, gs, b/b* and end-of-stream text operators. ADR 0009's every_multi_char_operator_parses_as_itself test exists to stop exactly this - and would have caught it, except BMC was one of two operators excluded from its table as "genuinely unimplemented". Excluding a known-broken operator from the test whose job is finding broken operators is how it survived. The exclusion list is gone. 2. BDC discarded its property list, which carries /MCID - the only link between a run of page content and the structure element describing it. Without it a tree can be parsed but never attached to anything. 3. The op produced by BDC was named MarkContentBmc, and BMC produced nothing. The names were the wrong way round, which is how the missing arm survived review: the enum looked like it had a BMC case. 4. Found while fixing 2: the content lexer had no dictionary support at all. It read `<` as a hex string without checking for a second `<`, so `<</MCID 0>>` parsed as the string "0C0D0". Content streams now parse direct objects properly, bounded at 16 levels; a single `<` is still a hex string and a test pins that. On top of that, new pdf-document/src/structure.rs: /StructTreeRoot, /StructElem trees, /RoleMap resolution, depth-first reading order, /Alt, /ActualText, /E, /Lang, and MCID-to-element lookup. Cycles in /K are cut at the first repeat and reported by object number rather than expanded to the depth bound. Accessibility findings are mechanical checks reported as findings, NOT a conformance verdict: there is no is_pdf_ua and no ComplianceReport, and a mutation-checked tripwire test fails if either appears. Real PDF/UA conformance needs human judgement - whether /Alt text is accurate is not mechanically decidable - so claiming it from six checks would be exactly the overclaim this codebase keeps removing. 7 corpus fixtures, 14 acceptance tests, 22 unit tests, and a parse_content_dict fuzz target for the new object parser. TEST_TARGET=pdf 536 -> 575, TEST_TARGET=pdf-ui 580 -> 619. rustfmt and clippy -D warnings clean.	2026-07-31 22:19:45 +00:00
andodeki	e4a0c0a79e	feat(pdf): read signatures, and fix a third ObjRef-destroying resolve ... Phase 8 #6 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Signatures"). Design and merge criteria in REVIEWS/adr/0010-pdf-signatures.md. Two defects, and the first is not about signatures at all. 1. acroform() dereferenced /AcroForm with self.resolve(), which recurses. That replaced every /Fields [5 0 R] entry with an inline dictionary, so AcroForm::walk saw node.as_ref() == None, decided the field had no identity to key an edit on, and dropped it. The form came back EMPTY rather than wrong, which is indistinguishable from a document with no fields, so nothing announced the loss. This is the third appearance of one mistake: page_annotations once destroyed every annotation's obj_ref the same way, and extract_xobjects resolved a reference then asked the resolved object for as_ref(), leaving every page's XObject map empty. It was invisible because both existing fixtures declare /AcroForm as an INDIRECT reference, where only one level is dereferenced and the refs survive. A direct /AcroForm dictionary - equally legal - hits it. The new fixture uses one deliberately; reverting the one-line fix fails 9 of the 12 new acceptance tests. 2. Nothing read signatures. FieldType::Signature was classified and then ignored; /ByteRange, /Contents, /SubFilter and /DocMDP appear nowhere in the codebase. A signed contract was presented exactly like an unsigned one. New pdf-document/src/signature.rs reads the signature dictionary and checks BYTE-RANGE INTEGRITY, which needs no cryptography and catches the common real-world tampering: whether the signed range reaches the end of the file. A signature that stops short leaves appended bytes uncovered, which is exactly how an incremental-update attack hides content behind a signature that still verifies. Cryptographic verification is NOT implemented and cannot be faked: VerificationStatus has no Valid variant. That is enforced by the type, not by convention, because the failure mode for a signature feature is not "it doesn't work" - it is a green tick beside a document nobody checked. A test asserts the capability's absence so adding Valid without the cryptography breaks the build rather than shipping a false tick. Signing is out of scope entirely: no private keys in this crate. Also verified ADR 0003's claim that an append-only save preserves a signed byte range, byte for byte, rather than leaving it asserted. Implementation bug worth recording: /Contents was initially hex-decoded, but the COS lexer already decodes <...>. Running it twice on the common 128-zero-byte placeholder - which contains no hex digits - produced an EMPTY vector, silently discarding the signature blob while every other field looked right. Caught by asserting the blob is non-empty rather than asserting the parse returned Ok. 6 corpus fixtures, 12 acceptance tests, 28 unit tests, and a parse_signature fuzz target because /ByteRange is four attacker-controlled integers used to index the file. TEST_TARGET=pdf 497 -> 536 passing. rustfmt and clippy -D warnings clean.	2026-07-31 20:02:29 +00:00
andodeki	d8d29c226d	feat(pdf): transparency, and four operator-parsing bugs it exposed ... Phase 8 #5 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced transparency"). Design and merge criteria in REVIEWS/adr/0009-pdf-transparency.md. The headline defect was not that transparency was missing. `gs` was MIS-PARSED as CloseStroke: /GS0 gs 1 0 0 rg 100 100 200 200 re f -> [CloseStroke, RgbFill, Rectangle, FillWinding] so every page setting a graphics state gained a stroked path the document never asked for, and lost its alpha, blend mode and soft mask silently. Downstream everything was dead: StrokeExtGState/FillExtGState were never constructed, set_fill_opacity was never called and emitted no command when it was, and PdfPage::ext_gstate was read by no code at all. New pdf-graphics/src/transparency.rs: full /ExtGState (ca, CA, BM, SMask, LW, LC, LJ, ML, D, AIS, TK), all sixteen blend modes including the four non-separable ones, soft masks with /S, /G, /BC and a /TR evaluated through ADR 0006's PdfFunction, and /Group parsing. Wired end to end: parser -> PdfOp::SetExtGState -> device -> RenderCommand -> Makepad renderer. Compositing is NOT claimed. Backdrop blending needs render-to-texture, which an engine-neutral crate has no framebuffer for. Constant alpha is applied because it needs no backdrop; blend modes and soft masks are reported through TransparencyError::Unsupported rather than dropped, because a silently ignored /Multiply looks exactly like a correct /Normal. The ADR required a test enumerating every multi-character operator, on the grounds that fixing the third instance of a shadowing bug (after cm and rg) without preventing the fourth is not a fix. It immediately found three more live bugs, none of them transparency-related: - `b` and `b*` dropped their close-path, mapping to FillStroke* instead of CloseFillStroke*, so every closed-and-stroked path drew with a gap. - Text operators within three bytes of the end of a content stream were mis-parsed: the b'T' arm guarded `*i + 3 < len` while reading only two bytes, so a stream ending in `/F1 12 Tf` parsed as FillWinding. And the transparency-group fixture found a fourth: - PdfPage::xobjects was empty for every page of every document. extract_xobjects called doc.resolve(), which follows the reference, then asked the resolved object for as_ref() - always None - and only accepted a bare dict when every XObject is a stream. No `Do` operator could be resolved through the page model. Same defect as the one that once destroyed annotation object references; it now has its own regression test. T* and BMC are genuinely unimplemented and are deliberately excluded from the guard's table rather than papered over. 7 corpus fixtures, 12 acceptance tests, 32 unit tests asserting the §11.3.5 formulas (not our own output), and a parse_ext_gstate fuzz target. TEST_TARGET=pdf 451 -> 497, TEST_TARGET=pdf-ui 495 -> 541. rustfmt and clippy -D warnings clean.	2026-07-31 18:58:31 +00:00
andodeki	00f1dfbc12	fix(pdf): fuzz every target, and close two ADR 0004 gaps ... Three defects found by auditing the ADR merge criteria against the code rather than against memory. 1. The scheduled fuzz job ran five hardcoded targets. Four had been added since and were never fuzzed: parse_revision_chain, decrypt, eval_function and parse_colorspace. eval_function is the sharpest of those - it executes PostScript taken verbatim from an untrusted file. The list now comes from `cargo fuzz list` and the job fails rather than passing vacuously if it comes back empty. `cargo fuzz list` reads the manifest, so a target file added without its [[bin]] entry would still be skipped silently. The engine job, which runs on every push, now checks the two agree. Both directions of that guard were exercised before committing. 2. ADR 0004 rule 3 promises an annotation whose appearance cannot be generated "keeps its original /AP and is reported as skipped". The keeping worked - to_dict clones the source dictionary - but nothing reported it: SaveReport only tracked skipped appearances for form fields. A caller who moved a stamp was never told its artwork still showed the old position. Adds SaveReport::annotation_appearances_skipped and appearance_is_generated, which enumerates the out-of-scope types explicitly so a new AnnotationType fails to compile until classified. 3. SetContents and SetFlags had no round-trip test. Both were implemented and unit-tested against the in-memory model, but neither was ever reparsed from written bytes - the assertion ADR 0004 calls central. New fixture annotations/stamp.pdf carries real /AP artwork for a Stamp (undrawable: must be preserved and reported) beside a Square (drawable: must not be reported), so the reporting cannot pass by reporting everything. The stamp test was mutation-checked: it fails when the reporting line is removed. ADR 0003 and 0004 merge criteria are now ticked. 0003's were genuine paperwork - every box traced to an existing named test. 0004's were not, and its ADR now records what was missing rather than implying it always worked. TEST_TARGET=pdf 447 -> 451 passing. rustfmt and clippy -D warnings clean.	2026-07-31 18:34:25 +00:00
andodeki	eb3edde51b	docs(valhalla): add deep architectural comparison of valhalla-rs vs makepad map navigation layer	2026-07-31 18:32:14 +00:00
andodeki	b5e38825fa	fix(pay): validate money at the persistence boundary (B6) ... B6 is the last unfinished P0 on the review's priority table. ## Why the boundary and not the format The obvious reading of B6 is "change f64 to Money on disk", which is a data migration and belongs with the SQLite move. But measuring first shows the stored representation is not where the damage is: - The f64 round trip is exact. 1500.0, 1500.5, 0.1+0.2, 1e20 and 12345678.995 all write and re-read bit-identically. - The read path is the damage. parts[2].parse().unwrap_or(0.0) turned any unreadable amount into a confident KSh 0 row. - And NaN gets in. "NaN" and "inf" both parse as f64 and the writer emits them back verbatim, so they survive a round trip. One corrupt SMS then poisons every total it enters, permanently — once a NaN is in a sum, every comparison against that sum is false. ## What changed validate_money refuses three classes, on parse, on load and on save: non-finite; negative (direction lives in TransactionType, so a negative amount is a contradiction); and beyond 2^53, where f64 can no longer represent consecutive shillings. A row whose amount cannot be trusted is skipped with a log line rather than zeroed. Dropping a row is visible and recoverable by rescanning the inbox; a silent KSh 0 is neither. balance and cost are optional context, so they degrade to None rather than discarding the row, but can no longer be NaN. The writer refuses to persist an invalid amount, which is what stops a poisoned value becoming permanent. Smaller parse fix: "Ksh ,5" used to read as 5. A separator before any digit means the text is not the expected shape, and guessing is worse than declining. ## The parser had no tests parser.rs — the file deciding what every observed amount is — had zero tests. It now has 13: validation, extraction, end-to-end parsing, and unicode/NUL bodies that must not panic on a byte-index slice. M-Pesa harness: 7 -> 24 tests. ## Verifying the tests can fail validate_money was reduced to Some(value) and the harness re-run: 4 tests failed. A guard that cannot fail is decoration. ## Validation mpesa harness: 24 tests pass domain : 137 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 / batch / settlement-tick guards pass defect injection: 4 tests fail with validation removed pass ## What B6 still leaves open MpesaTransaction::amount is still f64 on disk. That is deliberate and now bounded: nothing untrustworthy can enter or leave the store, so the remaining exposure is precision within the validated range, which for whole-shilling amounts under 2^53 is none. Converting the field means rewriting the PSV format and migrating existing files. ADR 0002 records B6 as partially addressed with the migration deferred to the SQLite move.	2026-07-29 04:48:50 +00:00
andodeki	cf878d9e3e	feat(pay): coordinator entry point for an externally granted authorization ... The API change tranche 10 scoped, in REVIEWS/adr/0007. ## dispatch_with_authorization Tranche 10 established that Android cannot implement BiometricAuthorizer without reopening defect S2, and built AuthorizationAttempt as the replacement. It named the remaining work: a coordinator entry point taking an already-granted attempt instead of calling biometric.authenticate(). coordinator.dispatch_with_authorization(&id, &mut attempt) It does not prompt, does not wait, and does not consult the injected BiometricAuthorizer at all. That is asserted rather than assumed: one test injects an authorizer reporting no hardware that also errors, and shows a granted attempt still dispatches. If the coordinator ever fell back to it, that test fails. Three refusals, each with a test that fails when the gate is removed: - An unanswered prompt does not dispatch. PromptShown plus SensorEngaged is not consent — the exact shape the Android adapter would produce. - A grant belongs to one payment. A foreign grant is refused before anything else, so a stray callback cannot even fail the payment on screen; the victim intent stays in AwaitingUserAuthorization rather than being transitioned to Failed by a stranger. - A grant is spent on use. One authorization, one dispatch (B3). Denied, still-prompting and already-spent attempts all fail the intent closed and never reach the gateway. ## Verifying the tests can fail The gate was removed (if !attempt.consume() -> if false) and the suite re-run: 5 tests failed. A fail-closed test that passes against fail-open code is worthless, so this is the check that matters. Worth recording: a_grant_cannot_dispatch_twice still passed with the gate removed, because the existing duplicate-dispatch budget caught it independently. Two unrelated mechanisms refuse the second dispatch. That is defence in depth working, and the reason that test is not sufficient evidence on its own. ## Validation domain : 137 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 / batch / settlement-tick guards pass fail-open injection: 5 tests fail with the gate removed pass Domain tests 129 -> 137. ## What remains The authorization half is done and verified. PayFlowHandler still owns the USSD session lifecycle, the pending-store writes that shadow the coordinator's repository, and the bulk queue plumbing. Moving those makes the coordinator own the gateway session, which changes who cancels on teardown and who observes an out-of-order callback — ADR 0007's device matrix, which cannot be exercised here.	2026-07-29 04:10:59 +00:00
andodeki	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.	2026-07-29 03:57:07 +00:00
andodeki	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.	2026-07-29 02:55:49 +00:00
andodeki	a6e8f4c04e	fix(pay): remove the last false-success labels, and fix B6 where it is real ... 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.	2026-07-29 02:45:32 +00:00
andodeki	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.	2026-07-29 02:27:44 +00:00
andodeki	d0f5e74435	feat(pay): Phase 5 platform gateway boundary, and verify Phase 4 ... 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.	2026-07-29 02:07:19 +00:00
andodeki	7d21532ebf	feat(pdf): real colour spaces, ICC profiles and PDF functions ... Phase 8 #4 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md ("Advanced colour"). Design and merge criteria in REVIEWS/adr/0006-pdf-advanced-color.md. The interpreter tracked only the *name* of the active colour space and then passed sc/scn operands to the device as if they were already RGBA. Every non-device space therefore rendered a confident wrong colour with no error: /Spot cs 1.0 scn full tint of a spot ink -> pure red /Idx cs 3 scn palette entry 3 -> near-black /Lab cs 50 0 0 scn mid gray -> white (clamped) /DevN cs (5 inks) five colorants -> inks 5+ discarded /ICCBased profile-defined colour -> profile discarded DeviceCMYK also used the additive 1-c-k conversion, which crushes any colour printed over black. Three new modules in pdf-graphics: - function.rs PDF functions, all four types. Type 4 runs on a bounded interpreter: depth 32, 32768 tokens, stack 100, 100000 steps, and an unknown operator is an error rather than a no-op that would leave a plausible wrong colour. - icc.rs ICC matrix/TRC and gray kTRC profiles, applied exactly. LUT-class profiles are reported as such and the caller falls back to /Alternate; they are never pretended to be matrix profiles. - colorspace.rs All eleven families, converting through XYZ with Bradford adaptation and a real sRGB transfer function. Wiring: - PdfDevice gains set_stroke_components/set_fill_components, so SC/SCN reach the device as components of the active space instead of being read positionally as RGBA. - cs/CS now resets to the space's initial colour (table 74), which is why golden/colors.txt gains a line. - PdfPage::color_spaces carries /Resources /ColorSpace fully dereferenced with streams decoded; a half-resolved space would make every ICC profile, palette and type 0/4 transform silently fall back. - A space that cannot be resolved keeps the previous colour and records a typed ColorError. No colour is invented, and no error is swallowed. Tests: 12 corpus fixtures under tests/corpus/color/, 14 acceptance tests in pdf-document/tests/color.rs asserting numeric RGB (the broken code produced a colour for every one of these; only the value was wrong), plus unit tests per function type and per curve type. Two fuzz targets added: eval_function and parse_colorspace. TEST_TARGET=pdf 387 -> 447 passing, TEST_TARGET=pdf-ui 431 -> 491. rustfmt and clippy -D warnings clean.	2026-07-28 20:48:55 +00:00
andodeki	e2df12da90	feat(map): add RouteRenderPass to RenderGraph and document rider map integration plan	2026-07-28 18:56:27 +00:00
andodeki	147ca7de23	test(pdf): prove the AES-256 path and harden malformed encryption ... Closes the gaps in ADR 0005's own merge criteria. The encryption commit shipped with two of its stated criteria unmet, which an audit of the ADR checklist against the corpus caught: - "An AES-256 (/R 6) document opens likewise" had no fixture. The revision 6 key derivation and the AES-256 stream path were implemented and their helpers unit-tested, but neither had ever decrypted a real file. That is exactly the "asserting Ok proves nothing" trap the same ADR warns about, since a key-derivation error can produce plausible output for one algorithm and garbage for another. - "Malformed encrypted fixtures never panic" had no malformed fixtures at all; only well-formed documents were covered. New fixtures, generated by the checked-in script from the specification so they test agreement with the spec rather than with the reader: - encrypted/aes256.pdf, a /V 5 /R 6 document with an empty user password, full /U, /UE, /O and /OE entries and an AESV3 crypt filter. It decrypts, so derive_key_r6, the iterated SHA-256/384/512 hash and the zero-IV unwrap of /UE are now proven end to end rather than in isolation. - encrypted/truncated_u.pdf, a /U shorter than the 48 bytes revision 6 requires, which must be reported rather than indexed past the end. - encrypted/missing_o.pdf, an /Encrypt dictionary with no /O. - encrypted/absurd_length.pdf, a /Length of 999999 bits, which must clamp rather than panic or over-index. All four behave correctly: the AES-256 document decrypts to its marker, and the three malformed ones are refused with typed errors naming the offending entry. Encryption tests go from 13 to 17. The ADR's merge criteria are now ticked, with a note recording that the original commit shipped with the revision 6 path unproven and that this follow-up is what closed it. Not done here: the decrypt fuzz target could not be re-run. The nightly toolchain now available (2026-07-27) fails to build the cc crate that libfuzzer depends on, with errors inside cc itself rather than in this code. The target still compiles under stable and the earlier run of 1,953,940 executions stands; this is recorded rather than quietly skipped. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (387 tests) Both rustfmt and clippy -D warnings clean.	2026-07-28 17:50:44 +00:00
andodeki	01e16b6383	feat(pdf): implement encryption (Phase 8) ... Phase 8 feature 3 of 10, designed in REVIEWS/adr/0005-pdf-encryption.md. An encrypted PDF did something worse than fail: it succeeded. Probing a structurally valid RC4-encrypted file through the parser gave parsed OK: pages=1 page 0 content bytes=44 content parsed into 0 ops No error and no warning. The document reported a page, the page reported content, and the content interpreted to nothing because it was ciphertext. The user saw a blank page and was told the file was fine. That is the defect class Phase 0 existed to remove, and it was the worst one left in the PDF stack because it was silent. New pdf-cos/src/encrypt.rs implements the standard security handler for reading: - V1/R2 RC4 40-bit, V2/R3 RC4 40 to 128-bit, V4/R4 crypt filters selecting RC4 or AES-128, and V5/R6 AES-256 with the SHA-256 based revision 6 hash. - The empty user password, which is the common case for a document encrypted only to set permissions, and explicit user or owner passwords. The owner path recovers the user password from /O and re-derives. - Per-object keys, as the spec requires. Reusing one keystream across objects would be a real cryptographic break, so the object and generation numbers are mixed in by construction and a test asserts the keys differ. Every primitive comes from audited RustCrypto crates: aes, cbc, rc4, md-5 and sha2, all MIT OR Apache-2.0, which deny.toml already permits. Phase 0 deleted a hand-rolled MD5/SHA/AES/RC4 implementation from this codebase and called it a CVE factory; ADR 0005 keeps that rule. Refusals rather than half-open documents: a public-key or otherwise unsupported handler is refused and named, an unsupported V/R combination is refused, and a wrong password returns a distinct error so a caller can prompt again rather than reporting a damaged file. Permissions are parsed and exposed but deliberately not enforced, and the code says why: once content is decrypted a caller can read it regardless, so enforcing here would imply a guarantee that does not exist. Saving an encrypted document stays refused, as ADR 0003 established. Decrypting and then writing plaintext would silently strip the protection the author applied, which is not a decision a library should make. Fixtures: tests/corpus/encrypted/ gains RC4 40-bit, RC4 128-bit, AES-128 and an unsupported-handler document. The generator implements the handler's algorithms independently from the specification, so a fixture that decrypts shows the reader agrees with the spec rather than merely with itself. Each plaintext contains a marker the tests assert on, and one test additionally asserts the decrypted content interprets to real render commands, because asserting Ok from the parser is exactly what the old broken behaviour did. Fuzzing: adds a decrypt target covering key derivation, which consumes attacker-controlled /O, /U, /P, /Length, filter names and file id. Run for real rather than compile-checked: 1,953,940 executions, no crashes. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (383 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (427 tests, 6 ignored) Both rustfmt and clippy -D warnings clean.	2026-07-28 17:39:38 +00:00
andodeki	fead12214b	docs(valhalla): add 1:1 file-by-file audit review and full port specification	2026-07-28 17:37:56 +00:00
andodeki	7eeba2884f	test(valhalla): expand unit and integration test coverage across all subcrates and add coverage analysis plan	2026-07-28 17:15:10 +00:00
andodeki	f04f8ba34e	feat(pdf): implement annotation editing (Phase 8) ... Phase 8 feature 2 of 10, designed in REVIEWS/adr/0004-pdf-annotation-editing.md. Its prerequisites are "document model, appearance generation, incremental save"; the first two landed in Phase 3 and the third in ADR 0003, so this was the ready one. Chosen ahead of the other unblocked feature, AcroForm full support, because the review lists that one as needing JavaScript actions. Running document-supplied code is a large new dependency and a security surface that deserves its own ADR and threat review rather than arriving as a side effect of finishing a form feature. Annotations were strictly read-only: the module had public fields and from_dict, and not one mutator or &mut self method. The viewer could report a click on a link but could not move a highlight, restyle a square or delete a stamp. New pdf-document/src/annotation_edit.rs, deliberately the same shape as DocumentFormEditor so a caller wiring a drag gesture does not have to learn a second contract: - AnnotationEdit covers Move, Resize, SetColor, SetInteriorColor, SetBorderWidth, SetOpacity, SetContents, SetFlags and Delete. - Every edit is validated before anything changes, so a rejected edit leaves the annotation untouched. Degenerate and non-finite rectangles, colours outside 0..1, negative border widths and out-of-range opacities are all refused with typed errors. - A degenerate rectangle is refused rather than silently normalised: it usually means a bug in the UI upstream, and quietly fixing it hides that. An inverted but valid rectangle is normalised on store, so hit testing and appearance sizing never see one upside down. - Read-only annotations refuse edits unless the caller opts in through an explicit allowing_read_only(), with one exception: clearing the read-only flag itself is permitted, or a locked annotation could never be unlocked. - Colour reading converts the grey and CMYK forms of /C to RGB, since the array length selects the space. Identity: PdfAnnotation gains an obj_ref, because an index into /Annots is not stable across a save. Populating it exposed a real bug in page_annotations: it called self.resolve() on the /Annots array, which recurses and replaced every entry with its dictionary, destroying the references. It now resolves only the array itself. Saving: save_annotation_edits appends a revision through the ADR 0003 writer. Deleting rewrites the page dictionary so the reference leaves /Annots, because an object that stops existing while the page still points at it produces a file other readers reject. Out of scope and recorded in the ADR rather than implied: creating new annotations, appearance generation for types this crate cannot draw (a Stamp keeps its existing /AP rather than being blanked), applying redactions, and rich text. Tests: 23 unit tests plus 13 corpus acceptance tests covering the ADR merge criteria. The round trips reparse from the written bytes rather than reusing in-memory state, assert a deleted annotation is gone from the reparsed page's /Annots and not merely from the model, that unrelated annotations survive a deletion, that two saves chain, and that editing then saving never panics on the malformed corpus. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (354 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (398 tests, 6 ignored) Both rustfmt and clippy -D warnings clean.	2026-07-28 17:12:40 +00:00
andodeki	4ab9a9b699	docs(valhalla): add 1:1 granular parity audit review and 4-phase completion execution plan	2026-07-28 16:47:34 +00:00
andodeki	d59bed5868	feat(pdf): implement incremental save (Phase 8) ... Phase 8 of REVIEWS/DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md, designed in REVIEWS/adr/0003-pdf-incremental-save.md. The review treats Phase 8 as ten independent projects, each needing its own design doc and merge criteria. Incremental save is taken first because it is the only one whose prerequisites are already met, it is listed as a prerequisite by two others (annotation editing and full AcroForm support), and it closes a real credibility gap: DocumentFormEditor has been able to edit form fields since Phase 3, and there was no way to save the result. A grep for a public save API across all four crates returned nothing. Design decision: append a revision, never rewrite. The original bytes are copied verbatim and changed objects are appended with a new xref chained through /Prev. A full rewrite would be easier and wrong: it would silently discard everything this parser does not yet model (structure trees, optional content, embedded files), and it would invalidate any signature, foreclosing a feature listed later in the same phase. ADR 0003 records this in full. Two latent bugs surfaced while building it, both pre-existing: - find_xref_start searched with windows(10) for the 9-byte keyword "startxref", so it never matched. Every parse silently fell through to a forward scan for the first "xref" in the file. On a single-revision document that happens to be correct; on an incrementally saved one it is the *oldest* revision, so a saved edit read back as its pre-edit value. This had no visible effect before because nothing produced multi-revision files. - XRefTable::parse read one section and ignored /Prev entirely, so a multi-revision document lost every object the earlier revisions defined. It now walks the chain newest-first, keeping the first definition of each object, with a visited set against /Prev loops and bounds checks on the offsets, which come from the file and cannot be trusted. A bad link ends the chain instead of indexing out of bounds. The xref unit fixture claimed startxref 408 in a 191-byte file and only ever passed because of the windows(10) defect; it is corrected rather than adjusted to keep passing. Implementation: - pdf-cos/src/incremental.rs: IncrementalUpdate builds one revision. Recomputes stream /Length so a caller cannot write an inconsistent one, emits xref subsections for contiguous runs, sizes /Size over the whole chain, and is byte-reproducible for a given set of edits. - pdf-document/src/save.rs: turns dirty AcroForm fields into a revision, writing the new /V and a regenerated appearance stream referenced from /AP, keyed by state name for checkboxes and radios. Refusals rather than partial saves: an encrypted document returns SaveError::Encrypted, because writing plaintext objects into it would corrupt the file; a source with no startxref or no /Root is refused; and a save with no pending edits returns the input unchanged rather than growing the file and churning its timestamp. Tests: 10 acceptance tests in tests/save_roundtrip.rs covering the ADR merge criteria. The central one reparses from the written bytes rather than reusing in-memory state, so it tests the file rather than the writer against itself. Also asserts three chained revisions still reparse with the newest value winning, that pages and annotations survive a save, and that saving never panics on the malformed corpus. Known limitations, recorded in the ADR rather than glossed: cross-reference streams and object streams are not written, and superseded objects are not compacted. Validation: TEST_TARGET=pdf ./tools/test-rust-clean.sh (307 tests) TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh (351 tests, 6 ignored) Both rustfmt and clippy -D warnings clean.	2026-07-28 16:35:22 +00:00
andodeki	7c5023ac85	docs(valhalla): add 1:1 complete port execution plan for remaining Valhalla subsystems	2026-07-27 16:41:47 +00:00
andodeki	eed01c0a55	docs(valhalla): add comprehensive parity and performance benchmark review vs C++ Valhalla	2026-07-27 16:30:21 +00:00
andodeki	ecaac4bcb7	feat(pay): complete Phase 3, start Phase 4 ... Phase 3 (secure repository) is now complete. 3.3 retention and erasure — new retention.rs: - Data minimisation and financial record keeping pull opposite ways, so the policy states both: evidence expires 90 days after settlement, settled records are held for a five-year floor, never-dispatched intents expire in 30 days, and anything awaiting reconciliation is never swept. Deleting an unresolved payment would destroy the only record of money that may have left the account. - erase_payment_on_request honours a user deletion request without destroying the accounting record: it normally erases the parsed copy of the user's inbox and retains the financial record, erases fully only past the floor, and refuses outright while in flight. Refusals carry a reason. 3.4 backup, restore and corruption reporting: - integrity_check and foreign_key_check surface damage as StorageError ::Corrupt rather than letting a damaged ledger be trusted. - backup_to uses SQLite's online backup API instead of copying the file, so a concurrent writer cannot produce a torn copy, and refuses to back up a database that fails its integrity check. - restore_from validates the candidate in a separate connection before touching the live database, so a failed restore cannot destroy a working ledger. Tested with both a missing and a corrupt backup. Storage tests 20 -> 36. Phase 4 (blocking I/O) — 4.1 to 4.3 implemented: - 4.1 expenses/mod.rs called reload(), a full parse of the store from disk, as the first statement of draw_walk; scrolling re-read the whole ledger every frame. Now loaded once, with invalidate()/reload_if_stale() on the event path. - 4.2 the SMS scan was a wall-clock check inside draw_walk, so the branch ran every frame and the scan landed mid-paint. Now a cx.start_interval timer on the event path, with the interval named rather than inline. - 4.3 update_summary walked the filtered list and rewrote labels every frame. Now invalidated only in refresh_filtered and applied in handle_event, fully out of the draw path rather than guarded inside it. - 4.4 already satisfied by PaymentStorageWorker since tranche 1. Supporting domain work: view_state.rs adds PeriodSummary and TransactionsViewState so the summary arithmetic is testable outside a widget. The tests immediately pin two things the inlined code got wrong: totals now report overflow instead of wrapping, and a net outflow returns None rather than a clamped zero. Money gains Default (zero). Domain tests 66 -> 75. Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt, clippy -D warnings and cargo-deny; storage also --features sqlcipher; domain benches run; 7 M-Pesa store tests pass; 49 manifests parse. NOT VERIFIED: the nigig-pay widget edits for 4.1-4.3 are not compiled here. That crate needs the git-hosted Makepad fork. cargo check -p nigig-pay on a full developer checkout is required before Phase 4 can be called complete.	2026-07-27 15:34:38 +00:00
andodeki	c6e122c3fc	feat(pay): complete Phase 1 and fix M-Pesa store defects B1/B4 ... 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.	2026-07-27 15:23:20 +00:00
andodeki	01bc055d1e	feat(pay): complete the partially-done review phases ... 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.	2026-07-27 15:07:28 +00:00
andodeki	70eddf42c7	feat(pay): add evidence, event correlation and explicit reconciliation ... Implements the review phases left open by the previous tranche in the two pure crates, per REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md. nigig-pay-domain: - evidence.rs: ObservedEvidence/EvidenceLog make SMS and USSD output an untrusted observation that can raise a reconciliation flag but can never settle a payment (0.4, B2). Replays are de-duplicated by a deterministic body digest; only the digest is retained, never the raw body (0.6). - reconciliation.rs: explicit queue with typed reasons and attributed resolutions (2.8, 6.3). SafeToRecreate never mutates or re-dispatches the original intent. The user-facing message says "do not retry" and never "failed" (U4). - coordinator.rs: operation_id -> intent index so an unmatched callback changes nothing (2.7); hard duplicate-dispatch guard where only a never-started dispatch refunds the budget (2.9, B3); expire_local_confirmation_window replaces Submitted -> Failed on timeout and cannot disturb a settled payment. nigig-pay-storage: - schema migration 2 adds payment_intent_evidence with a unique digest constraint and a durable payment_reconciliation_queue (3.2, 3.3). - legacy import now durably queues unprovable records. - fault-injection tests for unwritable path, corrupt file, unusable worker path and audit-insert rollback (3.4); restart-equivalence tests (8.2). Both crates deny unwrap/expect/panic/indexing outside tests (7.8). That ratchet caught PaymentStorageWorker::spawn aborting on thread-spawn failure; it is now fallible and returns StorageError. Validated in an isolated toolchain over copies of the two pure crates: cargo fmt --all --check clean, cargo clippy --workspace --all-targets -D warnings clean, 43 domain tests and 17 storage tests passing, and all 49 workspace manifests parse. The full workspace is not asserted buildable here: external Makepad/Robius sibling path dependencies are absent, so the Makepad UI wiring, SQLCipher/Keystore encryption and the Java PIN scrub remain explicitly unclaimed.	2026-07-27 14:39:15 +00:00
andodeki	f6750c8259	feat(pay): add domain coordinator and sqlite storage foundation	2026-07-26 18:42:02 +00:00
andodeki	cc05abdc71	Initial commit	2026-07-26 19:38:26 +03:00