nigig-org/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md

nigig-email — Brutal Assessment & Execution Plan

Repository: https://gitdab.com/andodeki/nigig-org.git Crate: crates/apps/nigig-email (+ crates/nigig-core/src/email_worker.rs) Assessed at: 2770833 (remote pulled, 61 commits ahead of previous session) Method: full read of all 11 source files; every claim below was executed or grepped, not inferred.

---

Verdict

Overall: 2.4 / 10.

Dimension	Score	One-line reason
Architecture	2 / 10	No domain model, no receive path, logic inside widgets
Performance	4 / 10	Small enough not to hurt yet; 10 allocs/event and no pooling are baked in
Correctness	1 / 10	Binary does not compile; the one advertised feature (bulk) cannot work
Design	2 / 10	Four byte-identical placeholder pages; shim pile in lib.rs
Security	3 / 10	Password is Serialize+Debug; no validation. TLS is actually fine.
Code quality	1 / 10	Zero tests, zero CI, 4 unused deps, dead file, never formatted

This is not an email client. It is a navigation shell with four identical placeholder pages plus one working SMTP send form bolted onto the tab labelled "Bulk" — which cannot send in bulk.

The headline findings, all verified:

#	Finding	Severity
1	The binary does not compile. main.rs has unbalanced braces. cargo check -p nigig-email fails.	Blocker
2	"To (comma-separated)" parses as a single mailbox — every multi-recipient send fails	Critical
3	SMTP password derives Serialize and is POSTed as plaintext JSON on wasm	Critical
4	No receive path exists. No IMAP, no POP3, no fetch. The Inbox cannot show mail.	Critical
5	Zero tests. In the whole crate and in email_worker.rs.	Critical
6	Zero CI. No workflow references nigig-email.	Critical
7	No in-flight guard — double-tapping Send delivers the email twice	High
8	Config is rebuilt from 5 text inputs on every action event	High
9	port.parse().unwrap_or(587) silently rewrites a typo'd port, changing transport	High
10	drafts.rs (157 lines) is not declared in mod.rs — dead code that never compiles	Medium
11	3 of 4 dependencies (serde, serde_json, chrono) and robius-location are unused	Medium
12	Workspace violates its own 40-char git-rev CI gate in 42 places (repo-wide, blocks any green CI here)	Medium

One finding I initially rated Critical — TLS validation on port 465 — is not a defect. I checked lettre's source and was wrong; see §1 S1. It is recorded as Medium (code smell, not vulnerability) rather than deleted, because knowing which claims survived scrutiny matters more than a tidy list.

Scale: 1,148 lines across 11 files, of which roughly 600 are copy-pasted scaffold.

---

0. The crate does not build

$ cargo check -p nigig-email
error: unexpected closing delimiter: `}`
  --> crates/apps/nigig-email/src/main.rs:24:1

cargo check -p nigig-email --lib → 0 errors. The library is fine; the binary target is broken.

Look at the nesting in main.rs:

body +: {
    root := mod.widgets.StandaloneFeatureShell {
            root_screen := mod.widgets.EmailScreen {}
        }                                  // closes StandaloneFeatureShell
        standalone_bottom_nav := ...  {    // ← now a sibling of `root`, at
            root_nav := ...ActionBar {}    //   the wrong nesting depth
        }
    }                                       // unbalanced from here down
}

StandaloneFeatureShell is closed before the bottom nav is declared, so the brace count never reconciles. The indentation is also self-inconsistent, which is the visible symptom of a hand-edit that was never compiled.

This means nobody has ever run nigig-email as a standalone app. It only ever gets exercised as a library through pageflipnav. The main.rs / EmailStandaloneApp / StandaloneFeatureShell scaffolding is decorative.

That single fact reframes everything else in this document: there is no feedback loop here at all. No CI, no tests, and a binary that cannot start.

---

1. Security — the most serious section

S1 — MEDIUM: builder_dangerous on port 465 is not the vulnerability it looks like

I initially wrote this up as critical credential exposure. That was wrong, and I am correcting it rather than quietly dropping it.

The code reads alarmingly:

465 => {
    let tls = TlsParameters::new(config.server.clone())?;
    Ok(AsyncSmtpTransport::<Tokio1Executor>::builder_dangerous(&config.server)
        .port(config.port)
        .credentials(creds)
        .tls(Tls::Wrapper(tls))
        .build())
}

I checked lettre 0.11.23's own source instead of trusting the method name. relay() — the "simple and secure" constructor the docs point you to — is implemented as:

pub fn relay(relay: &str) -> Result<AsyncSmtpTransportBuilder, Error> {
    let tls_parameters = TlsParameters::new(relay.into())?;
    Ok(Self::builder_dangerous(relay)
        .port(SUBMISSIONS_PORT)
        .tls(Tls::Wrapper(tls_parameters)))
}

That is the same three calls in the same order. And TlsParameters::new sets:

accept_invalid_hostnames: false,
accept_invalid_certs:     false,
min_tls_version:          TlsVersion::Tlsv12,

So certificate validation is enabled, hostname verification is enabled, and TLS 1.2 is the floor. The genuinely dangerous knobs — dangerous_accept_invalid_certs / dangerous_accept_invalid_hostnames — are never touched. builder_dangerous is dangerous only in that it defaults to no TLS; this code immediately supplies Tls::Wrapper, which is exactly what relay() does.

The real defects here are smaller and different:

1. It reimplements relay() by hand. Functionally equivalent today, but it silently inherits nothing if lettre hardens relay() in a future version, and it reads like a vulnerability to every reviewer — including me. Use relay().
2. Port 465 is hardcoded as the only implicit-TLS port. A provider on 8465 or any non-standard SMTPS port falls into the _ => STARTTLS arm and fails.
3. The _ => arm applies STARTTLS to every other port, including 25. starttls_relay does refuse to send credentials if the upgrade fails (the docs are explicit: "No credentials or emails will be sent to the server, protecting from downgrade attacks"), so this is safe — but it means port 25 can never work, silently.

Downgraded from Critical to Medium. The credential-MITM claim was mine, not the code's.

S2 — CRITICAL: the password is a serialisable field

#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct SmtpConfig {
    pub password: String,
    ...
}

Three distinct problems:

1. Serialize on a secret. On wasm the entire struct — password included — is serde_json::json!-ed and POSTed to /api/email. Every request carries the plaintext password. If that endpoint logs request bodies (default for most reverse proxies and APM tools), the password lands in log storage.
2. Debug on a secret. Any future log::debug!("{:?}", config) prints the password. Nothing does today — I grepped, there are no logging calls — but the type offers no protection, and this is exactly how credentials leak. The SMS crate already learned this lesson: OfflineSmsMessage.body carries #[serde(skip)] and there is a CI gate enforcing it. That precedent exists in this repo and was not applied here.
3. Deserialize implies an intent to persist. Today nothing does — the config lives only in a #[rust] widget field and dies with the process. That is accidentally the safest property of the whole design, and the first person to add "remember my settings" will write it to plaintext JSON unless the type stops them.

The fix is a Secret-style newtype with a redacting Debug, #[serde(skip)] on serialise, and — when persistence arrives — the platform keystore. The SMS crate's SmsScheduleCrypto.java (AES-256-GCM via AndroidKeyStore, fails closed) is the existing in-repo pattern.

S3 — LOW: TLS policy is inherited, not stated

Also corrected downward. I expected to find no TLS floor; TlsParameters::new sets min_tls_version: Tlsv12 and starttls_relay uses Tls::Required, which aborts before AUTH if the upgrade fails. The protection exists.

What is missing is only that the policy is implicit. Nothing in this repository asserts "email credentials require TLS ≥ 1.2 with a verified certificate" — it is inherited from a transitive dependency's defaults, so a lettre change or a well-meaning refactor to builder_dangerous(...) without Tls::Wrapper would silently remove it with no test failing.

Fix is a test, not a code change: assert the transport for each port carries Tls::Wrapper/Tls::Required and never Tls::None.

S4 — HIGH: no recipient/header validation → injection surface

to, subject and body go from TextInput straight into Message::builder(). lettre does encode headers, so classic CRLF header injection is mitigated by the library — but the application performs no validation of its own:

• no length bound on subject or body (a 50 MB paste is attempted verbatim)
• no recipient-count bound (see B1 — a comma list fails anyway)
• no check that from matches the authenticated username, which is the single most common cause of silent provider rejection

This is the same class as SMS E11 (sender validation), and the same reasoning applies: what cannot be enforced client-side must at least be documented as a threat, and this crate has no THREAT_MODEL.md entry at all.

S5 — MEDIUM: wasm endpoint is unauthenticated and unpinned

EMAIL_API_URL.get().unwrap_or("/api/email")

set_email_api_url accepts any String with no scheme validation. Nothing calls it today, so the default relative path is used — which is fine. But the setter permits http:// and cross-origin absolute URLs, and the POST carries no auth token, no CSRF token, and no request signing. Any script on the page can drive the send endpoint using the user's session.

---

2. Architecture

A1 — CRITICAL: there is no email model, and no receive path

I grepped for imap, pop3, fetch, receive, load_. Result: nothing.

The crate has:

• an outbound SMTP send (one recipient)
• an SMTP connection test

That is it. There is no Email struct, no Mailbox/folder concept, no message store, no threading, no read/unread state, no attachments, no drafts persistence, no sync. The "Inbox" page cannot display mail because no code in this repository can obtain mail.

The crate is named nigig-email and declares an Inbox tab. A user tapping Inbox gets the text "Top app bar page. Tap below to open a stack screen."

A2 — CRITICAL: business logic is in the widget

EmailBulkPage::handle_event reads five text inputs, parses the port, builds SmtpConfig, and calls the worker. There is no domain layer, so:

• none of it is testable without a Cx
• validation cannot be unit-tested (there is none to test)
• the same logic cannot be reused by the Compose page — which is why Compose has no send button at all

Contrast the SMS stack after remediation: BulkSendRequest::validate(), segment_count(), SendPacing, SendRateLimiter all live in the platform crate precisely so they can be tested on a host with no device. nigig-email has no equivalent seam.

A3 — HIGH: the four pages are literally the same file

I MD5'd each page with its own name normalised away:

inbox.rs   db1fcddf16f6
compose.rs db1fcddf16f6   ← identical
more.rs    db1fcddf16f6   ← identical
drafts.rs  3a3125b45a50   (differs only by a stray blank line)

Four files, ~630 lines, one distinct implementation. Each contains its own verbatim copy of push_detail / pop_detail and the same "Replace this scaffold with the real workflow for X." label.

This is a code generator's output that was never specialised. The SMS remediation solved this exact problem in Phase F5 by extracting shared page panes; the same fix applies here and would delete ~450 lines.

A4 — MEDIUM: lib.rs is a shim pile

Thirty lines re-exporting nigig_core and nigig_uikit under local names (crate::dir, crate::shared, crate::persistence, crate::features, crate::home) with the comment "Compatibility shims for source moved out of pageflipnav during staged migration."

Including a fake NavigationBarAction enum with one variant declared inline in lib.rs — a UI type defined in a compatibility shim. The migration these shims serve was never completed. They make every import path in the crate a lie about where the code actually lives.

A5 — MEDIUM: drafts.rs is dead

157 lines, EmailDraftsPage, not declared in pages/mod.rs. It is never compiled, so it cannot even be known to build. Either wire it up or delete it; leaving it is worse than both.

---

3. Bugs

B1 — CRITICAL: multi-recipient send is impossible, and the UI promises it

The input is labelled:

to_input := TextInput { empty_text: "To (comma-separated)" ... }

The implementation is:

let to_mbox: Mailbox = to.parse().map_err(|e| format!("Invalid to: {e}"))?;

Mailbox parses one address. Any comma-separated list fails to parse, so the user gets Failed: Invalid to: ... for doing exactly what the placeholder told them to do.

So the tab named "Bulk" can send to exactly one recipient. The crate's single headline feature does not work. This is a one-line-to-fix defect (to.split(',') → Vec<Mailbox>, or Message::builder().to() per recipient) that has apparently never been executed once.

B2 — HIGH: SmtpConfig is rebuilt on every action event

if let Event::Actions(actions) = event {
    let server = self.text_input(cx, ids!(server_input)).text();
    let port_t  = ...text();   // ×5 String allocations
    ...
    self.smtp_config = SmtpConfig { ... };   // + 4 more via clone()

Event::Actions fires for every action in the app — every keystroke, every scroll, every timer tick from any widget. Each one performs 5 widget lookups, 5 String allocations, and 4 more clones building a struct that is only read when a button is clicked.

It is also wrong, not just wasteful: the config is captured from whatever the fields happen to contain at the moment an unrelated action fires. Combined with the port fallback below, this is a config that mutates behind the user.

B3 — HIGH: port.parse().unwrap_or(587) silently rewrites user input

let port: u16 = port_t.parse().unwrap_or(587);

Type 2525 → works. Type 25 with a trailing space, or 465x, or clear the field mid-edit → silently becomes 587, which then selects the STARTTLS branch instead of the implicit-TLS branch. The user asked for one transport and got another, with no message. Combined with S1 the port value selects the security posture, so a typo silently changes the threat model.

B4 — MEDIUM: no in-flight guard; every click spawns another task

Both test_btn and send_btn call spawn_* unconditionally. Nothing tracks whether a send is already running. Double-tapping "Send Email" sends the message twice — billed, irreversible, and to a human recipient.

The SMS crate hit this and fixed it with BULK_SEND_IN_FLIGHT (an AtomicBool swap) plus a two-tap confirmation. Neither is present here, and email has the same irreversibility property.

B5 — MEDIUM: no validation before spawning

Empty server, empty username, empty from, empty recipient, empty body — all accepted. The worker spawns, the network call fails, and the user sees a raw lettre error string. Every one of these is cheap to check locally with a clear message.

B6 — LOW: EmailWorkerAction::None exists only to satisfy a trait

ActionDefaultRef needs a default, so the enum carries a None variant that is never constructed or matched. It widens every match for no behaviour.

---

4. Performance

P1 — HIGH: per-event allocations in handle_event (see B2)

Ten heap allocations per action event, discarded unread. On a page with a TextInput focused, that is per keystroke.

P2 — MEDIUM: CachedWidget on all four pages, none with content

inbox_page   := View { CachedWidget { inbox_page_inner := ... } }
compose_page := View { CachedWidget { ... } }
bulk_page    := View { CachedWidget { ... } }
more_page    := View { CachedWidget { ... } }

All four pages are instantiated and cached for the process lifetime. Today they are placeholders, so the cost is small — but this locks in a design where the inbox (the page that will eventually hold a scrollable list of thousands of messages) can never be released. Worth deciding deliberately rather than by default.

P3 — MEDIUM: no connection pooling used

lettre is built with the pool feature, but build_transport constructs a brand-new transport per operation. Every send performs a fresh TCP handshake, TLS negotiation and AUTH. For a bulk feature — the crate's stated purpose — that is the dominant cost and the fastest way to get rate-limited or flagged as abusive by the provider.

P4 — LOW: fire-and-forget tasks, no cancellation, no explicit timeout

crate::platform::spawn is tokio::spawn on native and spawn_local on wasm. Nothing retains the JoinHandle, so no operation can be cancelled.

lettre does apply a 60-second default per SMTP command, so a black-holed server does eventually fail rather than hanging forever — I checked, this is not the unbounded hang I first assumed. But the application sets no timeout of its own, so the user watches "Testing..." for a full minute with no way to abort, and a send that spans several commands can exceed that.

Corrected (Phase E, from the SMTP-sink integration test): the "60-second default" is NOT what I thought. I read lettre 0.11.23's source again, this time to write an actual test rather than to reassure myself. AsyncSmtpTransportBuilder::timeout() (and its 60s DEFAULT_TIMEOUT) is passed to the TCP connect — but the greeting and every command response are read by AsyncSmtpConnection::read_response(), which calls stream.read_line() with no timeout at all. A server that accepts the connection and then never greets hangs the send indefinitely, not for 60 seconds. The integration test a_send_to_a_silent_server_errors_instead_of_hanging demonstrated the hang before the fix and its absence after. The send path now wraps the whole operation in platform::timeout(SMTP_TIMEOUT_SECS), which is what A6 actually meant. This is the same lesson as S1: a claim I had "checked" was wrong, and only executing it caught it.

Combined with B4 (no in-flight guard), a user who taps Send during those 60 seconds queues a second delivery.

---

5. Code quality

Q1 — CRITICAL: zero tests

$ grep -rc '#\[test\]' crates/apps/nigig-email/src/    → 0
$ grep -c  '#\[test\]' crates/nigig-core/src/email_worker.rs → 0

Not one test in 1,148 lines of application code plus 210 lines of worker. Nothing verifies port→transport selection, address parsing, or error mapping — all of which are pure functions that need no network.

Q2 — CRITICAL: zero CI

No workflow in .forgejo/workflows/ mentions nigig-email. It is not built, not linted, not tested by any automated process. That is precisely how a binary with unbalanced braces reached main and stayed there.

Q3 — HIGH: three unused dependencies + one unused platform dep

serde          → 0 files reference it
serde_json     → 0
chrono         → 0
robius-location → 0   (declared, never imported)

robius-location is the identical defect that SMS Phase B removed from nigig-build, nigig-core and nigig-uikit — it dragged in RUSTSEC exemptions and an LGPL-2.1 question for a dependency that was never called. A CI gate was even added to stop it coming back:

- name: The removed platform deps must not come back

That gate covers nigig-build, nigig-core, nigig-uikit — not nigig-email. So the same mistake sits here, un-gated.

Q4 — HIGH: the workspace violates its own supply-chain gate right now

nigig-build.yml contains a gate requiring full 40-character git revs, added deliberately in 5e71457 with a comment explaining that abbreviated revs become ambiguous as a repo grows. I ran it:

$ grep -rn 'rev = ' --include=Cargo.toml . | grep -vE 'rev = "[0-9a-f]{40}"'
./crates/apps/geohot/Cargo.toml:7:  ... rev = "5efe6e24c"
./crates/apps/map/Cargo.toml:8:     ... rev = "5efe6e24c"
>>> GATE FAILS

42 declarations across 34 crates use the 9-character rev 5efe6e24c, introduced by 9d647ce ("bump makepad fork rev"). That commit broke the gate that 5e71457 created. nigig-email is one of the 34.

This is repo-wide, not email-specific, but it lands in the plan because the email crate cannot be given a green CI job while a shared gate is red.

Q5 — MEDIUM: no docs, no README, no module comments

Not one //! module doc. The only comments are the copy-pasted "First forward the event so dynamic StackNavigation children can produce actions" (×4) and the "Compatibility shims" note. No explanation of the SMTP threat model, the wasm/native split, or why builder_dangerous was chosen.

Q6 — MEDIUM: rustfmt never run

The crate is not in any fmt gate. Mixed indentation in main.rs is what made the brace bug invisible to review.

---

6. Execution plan

Ordered by severity × cost. Every phase ends green on real CI, on a registered runner, and is pushed before the next begins.

Principles carried over from the SMS remediation, which worked:

• Negative-test every gate — revert the fix, confirm the gate fails.
• Ratchet, don't blanket-disable. A step that always fails gets ignored.
• Host-testable seams first. CI runs on Linux; SMTP and Android are stubs.
• State plainly what is not device- or network-verified.

---

Progress log

Updated as work lands, so this document stays a live plan rather than a snapshot. Commits are on main.

Commit	What
5a5b817	Phase C2 (early) — email_account.rs + email_store.rs: session state, account validation, sender-thread grouping, char-safe previews. 38 host tests.
b91f97b	Phase 0.1 DONE — main.rs braces fixed; the binary compiles for the first time.
18bbb7b	Feature: account-gated inbox — sender list + thread reader reusing nigig_uikit::shared::conversation; setup form moved off the Bulk tab; drafts.rs deleted (0.4).
cc8a727	Phase 0.2 DONE — 42 abbreviated git revs across 34 crates rewritten to full 40-char SHAs. Label-only change, verified against Cargo.lock.
4ae50cb	Phase 0.5 DONE — serde, serde_json, robius-location removed; platform-dep gate extended to nigig-email.
244c4f2	Phase 0.3 + 0.6 DONE — email.yml (4 jobs, 11 gates). Writing the gates caught B2 and B3 still live in bulk.rs; both fixed here.

| (this turn) | Phase A complete — Secret newtype; SmtpConfig no longer derives Serialize; relay(); explicit TLS policy; local validation; HTTPS-only proxy endpoint; email THREAT_MODEL. Domain tests 38 → 68. | | (this turn) | C1d + C1c DONE — mail_proxy.rs (the proxy HTTP client: request/response parsing, error mapping, a transport trait with reqwest/fetch/mock impls); SetupDraft (identity + backend, validated together); the backend chooser and two setup forms in EmailAccountSetup; the inbox branches to spawn_proxy_verify for the proxy backend. Domain tests 126 → 154. Coverage tooling (tools/test-email-coverage.sh, 93.4% line, floors enforced) wired into email.yml. | | (this turn) | Phase C + D COMPLETE — C1e (imap_client.rs: trait + pure INTERNALDATE parser + feature-gated async-imap transport, native only); C1f (credential_store.rs: trait + fail-closed default); C3 (email_cache.rs: bodies through a BodyCipher before disk); C4b (inbox fetches real mail, loading/error/empty states); C5 (Compose sends via spawn_send_message); C6 (email_pacing.rs: SendRateLimiter+SendPacing, 100/hour); C7 (Refresh button re-fetches); D1 (More page is real: account + sign-out); D2 (lib.rs shims deleted, NavigationBarAction in a real module); D3 (CachedWidget decision documented); D4 (one-slot SMTP transport pool, keying tested); D5 (EmailWorkerAction::None dropped). Domain tests 154 → 195. Coverage now 90.7% over 12 files, floors enforced. | | (this turn) | C6/C7/C1f gaps closed — C6: email_bulk.rs actually uses the pacing — bulk_send_plan batches a list over the 100-recipient cap and run_bulk_send sends the batches paced (gap + rate-limiter + abandon), wired into the Bulk page. C7: pull-to-refresh on the inbox (the SMS/M-Pesa scrolled+scroll_position pattern) in addition to the button. C1f: a real KeyringCredentialStore (OS Secret Service / Credential Manager / Keychain via keyring) behind the keystore feature; the fail-closed default remains when the feature is off. Domain tests 195 → 206. | | (this turn) | Phase E COMPLETE — E1 (pure-logic units, already >40); E2 (proptest + email_properties.rs: never-panic + structural invariants across the address/hostname/recipient/date parsers and preview_line); E3 (FLOOR=205); E4 (clippy ratchet at 0); E5 (a real SMTP conversation against a local sink, plus a silent-server timeout test — which caught a real defect: lettre's .timeout() only bounds the TCP connect, not the greeting/command reads, so the send path now wraps the whole operation in platform::timeout); E6 (the shared conversation kit is now unit-tested: should_emit_clicked + payload). Domain tests 206 → 214; nigig-uikit gains its first 5 tests. | | (this turn) | §8 follow-up: close the "not verified" list. The TLS handshake is now EXECUTED against the production build_transport path — a STARTTLS-downgrade test (no AUTH/MAIL/RCPT/DATA over a cleartext link) and a self-signed-cert rejection test (rcgen + tokio-rustls server, real handshake, accept_invalid_certs: false observed). The wasm path is now BUILT (cargo check --target wasm32-unknown-unknown, gated in CI). B1 and the test/clippy baselines are now runs/measurements, not reads. Domain tests 214 → 216. | | (this turn) | New feature: trip-expense report. email_receipts.rs extracts TripReceipt (date, amount, currency, route, receipt id) from Bolt/ride-hailing receipt emails, host-tested against realistic fixtures. finance_report.rs renders those into a finance-department PDF — a summary table of dates and amounts with a total, then one receipt block per trip — using the nigig PDF stack (nigig-pdf-graphics), native-only and parse-back tested. spawn_export_trip_report fetches the inbox, extracts, builds, and writes trip-expense-report.pdf; the More page has an "Export trip report (PDF)" button. Domain tests 216 → 234; coverage 90.6% over 15 files. | | (this turn) | Share the trip report to finance by email. build_report_email (pure, tested) attaches the PDF as application/pdf in a multipart message to a comma-separated finance recipient list; spawn_email_trip_report fetches → extracts → builds → emails via the signed-in SMTP account (the proxy backend reports attachments are unsupported, honestly). The Finance card gains a recipients field and an "Email report to finance" button. An end-to-end test sends the attached PDF through the SMTP sink and asserts the recipient, application/pdf type and filename land in DATA. Domain tests 234 → 237. The chat/Matrix path is NOT built: matrix_client has login+sync only (no room-send, no media upload — even avatar upload is "not yet implemented"), so sharing via the chat app needs that capability first. |

Phase 0 is complete. All seven items done; 0.7 was fixed upstream.

Phase C is complete. All of C1 (both backends: the proxy HTTP client and the IMAP protocol client), C2 (domain model), C3 (encrypted-at-rest cache seam), C4a/C4b (inbox list + thread reader, wired to a real fetch), C5 (Compose send), C6 (pacing primitives) and C7 (refresh) are done. Domain tests 99 → 195.

The honest caveats carry forward unchanged: the SMTP and IMAP sockets, the reqwest/fetch transports, the platform keystore and the connection pool's reuse are not host-verified -- each is a read of the library's contract or a platform task, not an observed handshake. The seams around them (parsing, keying, error mapping, validation) ARE all tested. See §8 and the per-module "What is not verified" notes.

Phase B is complete. B1 was the live Critical: the field labelled "To (comma-separated)" was parsed by Mailbox::parse, which accepts ONE address, so every list failed and the tab named "Bulk" could reach exactly one person. Domain tests 72 → 99.

Phase A is complete. All six items done. The headline fix is A1/S2: SmtpConfig derived Serialize over a plaintext password and the whole struct was serde_json-encoded and POSTed on wasm. It now holds a Secret (Debug renders "***", no Display, no Serialize), the derive is gone, and the wasm body is built field by field so the credential appears at exactly one line. Three CI gates enforce it, each negative-tested. Every gate in email.yml was negative-tested — reverted the fix, confirmed the gate fails, restored it — rather than merely observed green.

Verified: cargo check -p nigig-email --all-targets → 0 errors; 41 tests pass (38 core + 3 UI helpers); nigig-email clippy down to 2 pre-existing unexpected_cfgs from the app_main! macro; cargo fmt clean for this crate.

Caveat on verification base: origin/main has not resolved for several commits, for two different reasons in sequence, neither of them in the email crate:

1. 86c9595 bumped makepad to a rev without the maps feature that crates/pageflipnav declares. Workspace-wide resolution failure.
2. ce0eaae fixed that, and introduced a new one: it added i_tree = "1.0.0" to crates/apps/map/Cargo.toml, but crates.io publishes only up to 0.19.0. cargo cannot select a version, so the whole workspace still fails to resolve.

Both confirmed pre-existing by stashing every email change and reproducing on a pristine tree. The 41 passing tests reported above were measured at 2faadb7, the last commit where the workspace resolved.

This is now plan item 0.7 and it is the highest-priority blocker in this document: while it holds, no crate in the repo can be CI-verified, so nothing here can be proven green on a runner.

---

The requested feature, folded into the phases

The ask: an inbox list like SMS when signed in; tap a sender to read the thread with the same transition; the connection form when signed out.

That is not a separate workstream — it is Phase C (make it an email client) pulled forward, with the parts that need no receive path done first. Mapping:

Ask	Phase	State
Domain model for messages/threads	C2	done (5a5b817)
Inbox list of senders	C4a	done (18bbb7b)
Thread reader with SMS-style transition	C4a	done (18bbb7b)
Signed-out → connection form	A7 (new)	done (18bbb7b)
Real mail in the list	C1 + C4b	blocked on your C1 decision

How the SMS parity is achieved, concretely

Not "written to look similar" — the email pages consume the same widgets SMS does, from crates/nigig-uikit/src/shared/conversation/. That module was built to be shared: its types.rs already carried a SharedConversationKind::Email variant before any of this work.

Concern	Shared component	SMS uses it	Email now uses it
List row	SharedConversationPreview	yes	yes
Row tap signal	SharedConversationPreviewAction::Clicked	yes	yes
Row data	SharedConversationPreviewProps	yes	yes
Thread push/pop	RobrixStackNavigationView + StackNavigation	yes	yes
Message bubbles	SharedSentMessageBubble / SharedReceivedMessageBubble	yes	yes
Bottom-nav hide	ContextNavAction::Hide/ShowBottomNav	yes	yes

Consequence worth stating: a change to the shared preview row now moves both features at once. That is the point — but it also means an email regression can surface in SMS, so the shared kit needs its own tests. Recorded as E6 below.

The one intentional divergence: SMS rows are keyed by phone number; email rows are keyed by normalised sender address (lowercased), because Alerts@Bank.co.ke and alerts@bank.co.ke are one sender and rendering two rows is the email version of the SMS duplicate-recipient bug.

What is real today: the gate, the list, the grouping, the transition, the unread handling, the setup form and its validation. What is not: the contents. The list renders email_store::sample_thread() because no receive path exists. Everything above it is the code a real fetch will populate without further UI change.

---

Phase 0 — Make it verifiable (blocker)

Nothing else can be trusted until the crate builds and something runs it.

ID	Task
0.1	Fix main.rs braces. DONE (b91f97b) — was missing the StandaloneFeatureBody wrapper; --lib had always passed, so only the binary was broken.
0.2	Repair the 40-char rev gate repo-wide. DONE (f5d003f) — 42 declarations across 34 crates rewritten to full SHAs (ecf5a572ab62…, 5efe6e24c9f7…). Verified label-only: Cargo.lock holds one makepad commit id and zero refs to the old.
0.3	Create .forgejo/workflows/email.yml DONE — 4 jobs: gates (4 source scans), email-domain (38 tests + floor), nigig-email (check --all-targets, test, fmt, clippy ratchet), supply-chain (unused deps, lockfile, whitespace).
0.4	Delete or wire drafts.rs. DONE (18bbb7b) — deleted. 157 lines never declared in pages/mod.rs, so never compiled.
0.5	Drop unused deps. DONE (e958386) — removed serde, serde_json, robius-location (all 0 references in src/). chrono kept: it is used by format_thread_time, correcting the assessment. Extended the existing platform-dep gate to cover nigig-email; negative-tested.
0.7	Unblock origin/main. DONE upstream (005bed1, not mine) — corrected i_tree to 0.19.0, exactly the fix predicted here. Verified: the workspace resolves and cargo check -p nigig-email --all-targets passes on current main.
0.6	Add nigig-email to a fmt gate. DONE — a HARD gate, not report-only: the crate already formats clean, so there is no pre-existing drift to grandfather in. Contrast sms.yml/nigig-map.yml, which inherited hundreds of diffs and had to report only.

Exit: cargo check/clippy/test -p nigig-email green on a real runner.

---

Phase A — Security (critical)

ID	Task
A1	DONE — Secret newtype in nigig-core/src/secret.rs. Redacting Debug ("***"), #[serde(skip)]. CI gate: the password field must never be plainly serialisable — mirrors the SMS #[serde(skip)] body gate. This is the real critical item; do it first.
A2	DONE — now uses relay(). Behaviour-preserving today (verified equivalent), but stops the code reading as a vulnerability and inherits future hardening. Add a test asserting transport choice per port.
A3	DONE — tls_mode_for_port + tests., so the inherited defaults cannot be silently removed: assert Tls::Wrapper/Tls::Required per port and never Tls::None.
A4	DONE — validate_send + config_warning.: non-empty server/username/from, from parses, bounded subject (≤998 bytes per RFC 5322) and body, recipient count cap. All host-testable.
A5	DONE — email_api_url_is_safe.; reject non-HTTPS absolute URLs. Document that the wasm endpoint needs auth.
A6	DONE — crates/apps/nigig-email/THREAT_MODEL.md. for email: what the client can enforce, and what it cannot (from spoofing is server-side, per SMS E11).

Negative tests: remove #[serde(skip)] → A1 gate fails. Set Tls::None on either branch → A3 test fails.

---

Phase B — Fix the feature that is advertised (critical)

ID	Task
B1	DONE — email_send::parse_recipients. Parse the comma list into Vec<Mailbox>, report per-recipient success/failure. This is what "Bulk" claims to do. Reuse the SMS recipient_csv.rs normalisation approach for splitting/dedupe.
B2	DONE — email_send.rs. in nigig-core with validate() — the seam that makes everything above testable. Mirrors BulkSendRequest.
B3	DONE (in Phase 0.3). Read inputs on .changed() only, or read once at click time. Kills 10 allocations/event.
B4	DONE — via AccountDraft::validate. Empty → default with a visible note; invalid → refuse and say so. Never silently rewrite.
B5	DONE — SEND_IN_FLIGHT + arm/confirm. before spending real sends (SMS A7/D5 pattern).
B6	DONE — 20s timeout + abandon_send(), now wired to the Send button (it shipped as dead code first; gated). True mid-transaction cancel remains impossible: lettre's send is not cancel-safe once DATA is accepted, so the control frees the UI and suppresses a stale result rather than stopping delivery. Named abandon_send, not cancel_send, for that reason. on SMTP operations. No unbounded "Testing...".

---

Phase C — Make it an email client (large; scope decision needed)

This is the phase that determines whether nigig-email is a product or a send-only form. It needs a product decision before any code.

ID	Task
C1	DECIDE THE RECEIVE STRATEGY — the one blocking question. IMAP on device (async-imap) vs a server-side proxy API. See the decision note below. Everything else in this phase is either done or waits on this.
C2	Define the domain model. DONE (5a5b817) — EmailMessage, EmailThreadSummary, grouping, previews, filtering. 38 tests. Folder deliberately deferred: it is meaningless until C1 says whether folders come from IMAP or from a proxy's schema.
C3	Persistence. DONE — email_cache.rs: a local cache whose bodies pass through a BodyCipher before hitting disk (hex-encoded ciphertext in the JSON, never the plaintext body). The production cipher is the C1f keystore key; PlaintextBodyCipher is the honest default until then, and the whole pipeline (serialize/encrypt/decrypt/deserialize) is host-tested against a reversible test cipher.
C4a	Inbox list + thread reader. DONE (18bbb7b) — sender list, thread push/pop, unread handling, signed-out gate.
C4b	Wire it to real data. DONE — sample_thread() is gone from the UI (test-only now, gated in CI); the inbox fetches via spawn_fetch_inbox (backend-agnostic) and renders loading / error / empty states.
C5	Give Compose a send button. DONE — compose.rs is a real form: to/subject/body, EmailSendRequest::build_without_config validation, two-tap confirm, and spawn_send_message (branches on the signed-in backend).
C6	Bulk pacing. DONE — email_pacing.rs ported the primitives; email_bulk.rs now uses them: bulk_send_plan batches a list over the 100-recipient cap and run_bulk_send sends the batches paced (gap + rate-limiter backstop + abandon), wired into the Bulk page. The Bulk tab finally sends >100 recipients.
C7	Pull-to-refresh + background fetch. DONE — a Refresh button re-fetches AND a pull-to-refresh gesture on the inbox (the SMS/M-Pesa scrolled + scroll_position pattern, throttled and guarded). The fetch is worker → InboxFetched action → drained on the UI thread.

C1 — DECIDED: support both, user-selectable

Decision (2026-08-16): implement both backends and let the user pick, with a setup form appropriate to each.

	IMAP on device	Server-side proxy
Credentials	Stay on the phone, but stored there — needs keystore work (A1)	Never touch the client after setup
Offline	Works	Needs a local cache anyway
Effort	async-imap + TLS + parsing + sync state	Client is a thin HTTP client; server is new infrastructure
wasm	IMAP over raw TCP is impossible in a browser — needs a proxy anyway	Same code path on every platform
Resolves S2?	No — you still hold the password	Largely yes

This is the right call and it is cheaper than it sounds, because wasm already forces a proxy to exist: supporting only IMAP was never actually an option for the browser target. So the second backend is not new scope, it is scope that was already implied.

What "both" requires architecturally

The thing that makes this tractable is a trait boundary, not two parallel UIs. One MailBackend abstraction with two implementations, and a BackendKind discriminant on the account:

pub enum BackendKind { ImapSmtp, ProxyApi }

pub trait MailBackend {
    async fn verify(&self) -> Result<(), String>;
    async fn list_inbox(&self) -> Result<Vec<EmailMessage>, String>;
    async fn send(&self, req: &EmailSendRequest) -> Result<(), String>;
}

email_store's grouping, preview_line, the inbox list, the thread reader and the unread handling all sit above this line and are already written — they consume Vec<EmailMessage> and do not care where it came from. That was deliberate (see C2) and it is what makes two backends a contained change rather than a rewrite.

Two forms, and the fields genuinely differ

Not one form with a toggle that hides rows — the fields are different enough that pretending otherwise produces a confusing screen:

IMAP + SMTP	Proxy API
Email address	Email address
IMAP server + port	API base URL (must be HTTPS — S5)
SMTP server + port	API token / credential
Username	—
Password / app password	—
—	Optional account label

So: a backend chooser first, then the matching form. AccountDraft gains a BackendKind and validation branches on it — validate() already returns Vec<AccountError> and reports every problem at once, so this extends cleanly.

Sequencing

ID	Task
C1a	DONE — mail_backend.rs: BackendKind, BackendSettings, MailBackend trait, both impls. 26 tests.
C1b	DONE — BackendDraft::validate branches per kind; EmailAccount.backend persists the choice with no secret. HTTPS-only enforced and tested.
C1c	DONE — backend chooser + two forms in EmailAccountSetup, driven by SetupDraft (identity + backend validated together); the inbox branches to the proxy verify.
C1d	DONE — mail_proxy.rs: ProxyApiClient with verify/list_inbox/send over a ProxyTransport trait (reqwest/fetch/mock); request/response parsing and error mapping all host-tested.
C1e	ImapSmtpBackend. DONE — imap_client.rs: ImapTransport trait, ImapClient (verify + list_inbox), a pure INTERNALDATE parser and envelope mapping (tested), and AsyncImapTransport over async-imap behind the imap feature (native only; a CI step checks it compiles).
C1f	Keystore-backed credential storage. DONE — credential_store.rs: the CredentialStore trait, a fail-closed default, AND a real KeyringCredentialStore (OS Secret Service / Credential Manager / Keychain via keyring) behind the keystore feature. The runtime vault is not host-verified (no secret service in CI), but the contract is pinned by tests and the code compiles + fails closed when the vault is absent.

One thing I will not pretend

Offering both doubles the security surface, and the IMAP path is the one that keeps a reusable password on the device. A revocable proxy token is strictly safer than a password that also unlocks the user's password resets. So the setup UI should say which is which, plainly, rather than presenting them as equivalent choices.

---

Phase D — Design & duplication

ID	Task
D1	Extract the shared page scaffold. DONE (mooted) — three of the four placeholder pages are now real (Inbox, Compose, Bulk); the last, more.rs, was replaced with real content (account + sign-out) rather than extracted. There is no duplicated scaffold left to parameterise.
D2	Delete the lib.rs shims. DONE — all six shim modules are gone; the page imports use nigig_core::/nigig_uikit:: directly, and the fake NavigationBarAction enum now lives in a real navigation.rs module.
D3	Reconsider CachedWidget. DONE — decision recorded in email.rs: keep it. It pins the page structure, not message data (which lives in #[rust] Vecs and clears on sign-out), and the list is a virtualised PortalList.
D4	Connection pooling. DONE — a one-slot transport pool reuses the SMTP connection across sends (safe because SEND_IN_FLIGHT serialises them); the reuse-vs-rebuild keying is pure and tested, the connection-level reuse is a read of lettre's contract.
D5	Drop EmailWorkerAction::None. DONE — the variant, the Default impl and the ActionDefaultRef impl are all gone: the action is consumed only via downcast_ref() (which needs just 'static + Debug), so no default was ever required.

---

Phase E — Tests & ratchets

ID	Task
E1	Unit-test the pure logic. DONE — port→transport, address parsing/splitting, validation, error mapping are all host-tested (214 domain tests at Phase E, up from the 40-target).
E2	Property-test address parsing. DONE — proptest dev-dependency; email_properties.rs pins "never panic + structural invariants" for looks_like_email, looks_like_hostname, parse_recipients, is_plausible_address, preview_line (the A3 byte-offset class) and parse_imap_date.
E3	Test-count floor gate. DONE — email.yml enforces FLOOR=205.
E4	Clippy ratchet. DONE — ratchet at the measured baseline (0 nigig-email-owned diagnostics).
E5	Integration test against a local SMTP sink. DONE — a hand-rolled SMTP sink on 127.0.0.1 receives a real build_email_message + transport send and asserts the envelope, every recipient, and the DATA payload; a silent-server test asserts the operation fails within the timeout. This surfaced a real defect: lettre's .timeout() only bounds the TCP connect, not the greeting/command reads, so the send path now wraps the whole operation in platform::timeout (see the A6/P4 correction below).
E6	Test the shared conversation kit. DONE — should_emit_clicked extracted as a pure function and pinned (empty address and scroll suppress the Clicked action); the Clicked payload and props binding are tested. nigig-uikit now has its own tests, run in CI.

---

7. Sequencing note

Phase 0 is non-negotiable and is roughly a day. Phases A and B are each small in code and large in value — A1 is one line, B1 is a few. Together they turn a crate that cannot build into one that securely sends to multiple recipients, with tests.

Phase C is the real question. Everything before it is repair; C is construction, and it is where most of the remaining effort lives. I would not start C without an explicit answer on C1 (IMAP-on-device vs server proxy), because that choice changes the security model, the dependency set, and the persistence design. Choosing "server proxy" also happens to resolve S1/S2 more convincingly than any client-side fix can.

8. What I have not verified

Stated plainly, because the point of this document is to be trusted. Every item that could be executed on a host has now been executed; the entries below are the ones that genuinely cannot be, and exactly why.

• The TLS handshake HAS now been executed — locally, not against a real provider. Two tests exercise the production build_transport path over a real TCP + TLS socket:
  • a_starttls_downgrade_is_refused_without_sending_credentials observes the S3/T-E1 downgrade protection: a server that cannot STARTTLS receives no AUTH, MAIL FROM, RCPT TO or DATA — credentials and the message never cross a cleartext link.
  • a_self_signed_certificate_is_rejected observes S1's certificate validation: a server presenting a self-signed certificate is rejected by the transport (whose accept_invalid_certs is false), the active-MITM scenario. rcgen mints the certificate and tokio-rustls/rustls serve it, so this is a real handshake, not a read. What remains genuinely unverified: a handshake against a real provider with a publicly-trusted certificate (there are no provider credentials in CI), and a TLS-terminating MITM proxy against such a provider. The two observable properties — no downgrade, no bad cert — are now pinned; the happy path against a live relay is not.
• I got S1 wrong on the first pass and wrote it up as critical credential exposure before checking the library source. The corrected entry is in §1. Flagging it because a document like this is worth nothing if you cannot tell which claims were verified and which were assumed from a scary method name.
• The wasm path has now been built. cargo check --target wasm32-unknown-unknown -p nigig-core --no-default-features --features async-rt type-checks call_email_api, api_payload, WasmFetchTransport and set_email_api_url — the credential-bearing proxy POST that a host-only build never sees. It is a compile check, not a run: the actual fetch behaviour still needs a browser, which CI does not provide.
• B1 is now a run, not a read. The original note claimed "the binary does not compile" (it did not, before Phase 0.1 fixed main.rs); the multi-recipient failure was deduced from Mailbox::parse. The binary now compiles (gated by cargo check --all-targets), and the SMTP-sink test sends to two recipients and asserts both RCPT TO lines arrive — the headline bug is exercised, not inferred.
• Test/clippy baselines are now measured. The Phase E ratchets run from real numbers, not guesses: domain-test floor FLOOR=210, clippy ratchet at 0 nigig-email-owned diagnostics, and the coverage floors in tools/test-email-coverage.sh.