16 commits

Author	SHA1	Message	Date
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	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	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	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	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	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	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