diff --git a/.forgejo/workflows/email.yml b/.forgejo/workflows/email.yml index 1bb7c72..14373ae 100644 --- a/.forgejo/workflows/email.yml +++ b/.forgejo/workflows/email.yml @@ -24,8 +24,13 @@ on: paths: - 'crates/apps/nigig-email/**' - 'crates/nigig-core/src/email_account.rs' + - 'crates/nigig-core/src/email_send.rs' - 'crates/nigig-core/src/email_store.rs' - 'crates/nigig-core/src/email_worker.rs' + - 'crates/nigig-core/src/mail_backend.rs' + - 'crates/nigig-core/src/mail_proxy.rs' + - 'crates/nigig-core/src/secret.rs' + - 'tools/test-email-coverage.sh' - 'Cargo.lock' - 'Cargo.toml' - 'rust-toolchain.toml' @@ -34,8 +39,13 @@ on: paths: - 'crates/apps/nigig-email/**' - 'crates/nigig-core/src/email_account.rs' + - 'crates/nigig-core/src/email_send.rs' - 'crates/nigig-core/src/email_store.rs' - 'crates/nigig-core/src/email_worker.rs' + - 'crates/nigig-core/src/mail_backend.rs' + - 'crates/nigig-core/src/mail_proxy.rs' + - 'crates/nigig-core/src/secret.rs' + - 'tools/test-email-coverage.sh' - 'Cargo.lock' - 'Cargo.toml' - 'rust-toolchain.toml' @@ -369,7 +379,9 @@ jobs: # --------------------------------------------------------------------- email-domain: runs-on: ubuntu-latest - timeout-minutes: 30 + # Two full compiles now (plain tests + the instrumented coverage run), + # so allow more than the pre-coverage 30 minutes. + timeout-minutes: 45 steps: - uses: actions/checkout@v4 @@ -395,15 +407,24 @@ jobs: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - name: Email domain tests - run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend::" + run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy::" # A floor, not a ratchet: these tests are cheap, pure, and the - # number should only go up. 38 today. + # number should only go up. 38 at Phase 0; 154 after C1c/C1d. - name: The email domain test suite must not shrink run: | set -euo pipefail - FLOOR=120 - out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: 2>&1)" + FLOOR=150 + out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: 2>&1)" + + # A coverage number that is only printed drifts down. This enforces a + # whole-domain floor plus per-file floors on the files that have + # actually harboured bugs (the SMTP password serialisation, the bulk + # tab that could not bulk-send, the proxy parser). The script is + # self-contained: its own rustup, cargo home and target dir, removed + # on exit. Same pattern as tools/test-pdf-coverage.sh. + - name: Coverage floors + run: ./tools/test-email-coverage.sh echo "$out" | grep -E '^test result:' || true n=$(echo "$out" | grep -E '^test result:' \ | sed -n 's/.* \([0-9]\+\) passed.*/\1/p' \ diff --git a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md index 78ff21b..e99d442 100644 --- a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -529,16 +529,18 @@ snapshot. Commits are on `main`. | `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`. | **Phase 0 is complete.** All seven items done; 0.7 was fixed upstream. -**Phase C is in progress.** C1a and C1b are done: the `MailBackend` trait -boundary exists with both implementations, `BackendKind` gates IMAP off -wasm (a browser cannot open a raw TCP socket), and the chooser exposes the -security difference rather than implying the two are equivalent. Domain -tests **99 → 126**. Next: C1c (the two forms in the UI), then C1d (proxy -backend first — smaller, only option on wasm, exercises the trait -end-to-end). +**Phase C is in progress.** C1a, C1b, C1c and C1d are done: the +`MailBackend` trait boundary and both validation halves exist, the chooser +and the two setup forms are in the UI, and the proxy backend is a real HTTP +client (verified against a mock transport, driven by `reqwest`/`fetch` in +production). Domain tests **99 → 154**. Next: C1e (the IMAP protocol +client, native only, gated behind a feature), then C1f (keystore-backed +credentials), C3 (encrypted persistence), C4b (wire the inbox to a real +fetch), C5 (Compose send button), C6 (bulk pacing), C7 (pull-to-refresh). **Phase B is complete.** B1 was the live Critical: the field labelled "To (comma-separated)" was parsed by @@ -761,8 +763,8 @@ extends cleanly. |---|---| | ~~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 | Backend chooser + two forms in `EmailAccountSetup`. | -| C1d | `ProxyApiBackend` — thin HTTP client. Do this **first** of the two: it is smaller, it is the only option on wasm, and it exercises the trait boundary end to end. | +| ~~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` — `async-imap`, native only. Gate behind a feature so wasm builds never pull it in. | | C1f | Keystore-backed credential storage (A1). Required for IMAP to be usable across restarts; optional for the proxy, which can hold a revocable token instead. | diff --git a/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs b/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs index 922cc6d..2aa18fd 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/account_setup.rs @@ -6,11 +6,18 @@ // account once, then you read mail. Leaving it on a tab called "Bulk" // meant the app had a credentials form and no way to see your inbox. // -// All validation goes through nigig_core::email_account::AccountDraft, -// which is unit tested on the host. This widget only moves strings. +// Phase C1c adds the backend chooser: the user picks how to connect, and +// the form changes to match. The two choices are NOT equivalent and the +// chooser says so plainly (see nigig_core::mail_backend::BackendKind: +// the direct backend keeps a reusable mailbox password on the device; the +// proxy backend uses a revocable token). +// +// All validation goes through nigig_core::mail_backend::SetupDraft, +// which is unit tested on the host. This widget only moves strings and +// flips between the two forms. use makepad_widgets::*; -use nigig_core::email_account::{guess_provider, AccountDraft}; +use nigig_core::mail_backend::{BackendDraft, BackendKind, SetupDraft}; script_mod! { use mod.prelude.widgets.* @@ -40,12 +47,53 @@ script_mod! { } Label { width: Fill, height: Fit - text: "Add an account to read your inbox and send mail. Your password stays on this device for the current session only." + text: "Add an account to read your inbox and send mail." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } } - form_card := RoundedView { + chooser_card := RoundedView { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + draw_bg +: { color: #xFFFFFF, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + + Label { + text: "How do you want to connect?" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 13.0 } } + } + + imap_choice := Button { + width: Fill, height: 44 + text: "Direct (IMAP + SMTP)" + draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 12.0, border_size: 1.0, border_color: #xD7E5FF } + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 11.5 } } + } + + proxy_choice := Button { + width: Fill, height: 44 + text: "Nigig mail service" + draw_bg +: { color: #xEEF4FF, color_hover: #xDDEBFF, color_down: #xCFE2FF, border_radius: 12.0, border_size: 1.0, border_color: #xD7E5FF } + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 11.5 } } + } + + chooser_selected := Label { + width: Fill, height: Fit + text: "" + draw_text +: { color: #x1C274C, text_style: theme.font_bold { font_size: 11.0 } } + } + + chooser_summary := Label { + width: Fill, height: Fit + flow: Flow.Right { wrap: true } + text: "" + draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } + } + } + + identity_card := RoundedView { width: Fill, height: Fit flow: Down spacing: 8 @@ -60,15 +108,48 @@ script_mod! { draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - Label { text: "SMTP server" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - server_input := TextInput { + Label { text: "Display name (optional)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + display_name_input := TextInput { + width: Fill, height: 42 + empty_text: "Shown to people you email" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + } + + // One form per backend. Only one is visible at a time; the + // chooser toggles them. + imap_form := View { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + draw_bg +: { color: #xFFFFFF, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + + Label { text: "IMAP server (reading mail)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + imap_server_input := TextInput { + width: Fill, height: 42 + empty_text: "imap.example.com" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "IMAP port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + imap_port_input := TextInput { + width: Fill, height: 42 + text: "993" + empty_text: "993" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } + } + + Label { text: "SMTP server (sending mail)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + smtp_server_input := TextInput { width: Fill, height: 42 empty_text: "smtp.example.com" draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - Label { text: "Port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - port_input := TextInput { + Label { text: "SMTP port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + smtp_port_input := TextInput { width: Fill, height: 42 text: "587" empty_text: "587" @@ -89,38 +170,67 @@ script_mod! { is_password: true draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } + } - Label { text: "Display name (optional)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - display_name_input := TextInput { + proxy_form := View { + width: Fill, height: Fit + flow: Down + spacing: 8 + padding: 18 + show_bg: true + visible: false + draw_bg +: { color: #xFFFFFF, border_radius: 22.0, border_size: 1.0, border_color: #xE2E8F0 } + + Label { text: "Mail service address" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + api_url_input := TextInput { width: Fill, height: 42 - empty_text: "Shown to people you email" + empty_text: "https://mail.example.com/api" draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - connect_btn := Button { - width: Fill, height: 48 - text: "Connect account" - draw_bg +: { color: #x1C274C, color_hover: #x2A3F6E, border_radius: 14.0 } - draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 14.0 } } + Label { text: "Access token" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + api_token_input := TextInput { + width: Fill, height: 42 + empty_text: "A token you can revoke" + is_password: true + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - setup_status := Label { - width: Fill, height: Fit - text: "" - draw_text +: { color: #xB4232C, text_style: theme.font_regular { font_size: 10.5 } } + Label { text: "Account label (optional)" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + label_input := TextInput { + width: Fill, height: 42 + empty_text: "e.g. Work, Personal" + draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } } + + connect_btn := Button { + width: Fill, height: 48 + text: "Connect account" + draw_bg +: { color: #x1C274C, color_hover: #x2A3F6E, border_radius: 14.0 } + draw_text +: { color: #xFFFFFF, text_style: theme.font_bold { font_size: 14.0 } } + } + + setup_status := Label { + width: Fill, height: Fit + text: "" + draw_text +: { color: #xB4232C, text_style: theme.font_regular { font_size: 10.5 } } + } } } } -/// Emitted when the user asks to connect. Carries the raw draft; the -/// screen validates and drives the connection test. +/// Emitted when the user asks to connect. Carries the raw setup draft +/// (identity + backend choice); the screen validates and drives the +/// connection test for whichever backend was picked. #[derive(Clone, Debug, Default)] pub enum EmailAccountSetupAction { #[default] None, - Connect(AccountDraft), + /// Boxed: `SetupDraft` carries a full `BackendDraft` (many Strings), so + /// it is large enough that clippy's `large_enum_variant` flags the + /// plain enum. + Connect(Box), } impl ActionDefaultRef for EmailAccountSetupAction { @@ -134,8 +244,12 @@ impl ActionDefaultRef for EmailAccountSetupAction { pub struct EmailAccountSetup { #[deref] view: View, - /// Set when the address field last auto-filled the server, so a user - /// who typed their own server is never overwritten. + /// Which backend the user has selected. Defaults to the direct + /// backend; the chooser flips it and swaps the form. + #[rust] + backend_kind: BackendKind, + /// Set when the address field last auto-filled the servers, so a user + /// who typed their own is never overwritten. #[rust] autofilled_server: bool, } @@ -148,30 +262,26 @@ impl Widget for EmailAccountSetup { return; }; - // Autofill the server for known providers as soon as the address - // has a recognisable domain. This is why the form is not five - // mandatory fields for a Gmail user. + // Backend chooser. + if self.view.button(cx, ids!(imap_choice)).clicked(actions) { + self.backend_kind = BackendKind::ImapSmtp; + self.update_chooser(cx); + } + if self.view.button(cx, ids!(proxy_choice)).clicked(actions) { + self.backend_kind = BackendKind::ProxyApi; + self.update_chooser(cx); + } + + // Autofill the servers for known providers as soon as the address + // has a recognisable domain. This is why the direct form is not + // six mandatory fields for a Gmail user. if self .view .text_input(cx, ids!(address_input)) .changed(actions) .is_some() { - let address = self.view.text_input(cx, ids!(address_input)).text(); - if let Some((server, port)) = guess_provider(&address) { - let current = self.view.text_input(cx, ids!(server_input)).text(); - // Only fill a blank field, or one we filled ourselves. - if current.trim().is_empty() || self.autofilled_server { - self.view - .text_input(cx, ids!(server_input)) - .set_text(cx, server); - self.view - .text_input(cx, ids!(port_input)) - .set_text(cx, &port.to_string()); - self.autofilled_server = true; - self.view.redraw(cx); - } - } + self.autofill_provider(cx); } if self.view.button(cx, ids!(connect_btn)).clicked(actions) { @@ -193,7 +303,10 @@ impl Widget for EmailAccountSetup { .label(cx, ids!(setup_status)) .set_text(cx, "Checking connection…"); self.view.redraw(cx); - cx.widget_action(self.widget_uid(), EmailAccountSetupAction::Connect(draft)); + cx.widget_action( + self.widget_uid(), + EmailAccountSetupAction::Connect(Box::new(draft)), + ); } } } @@ -205,14 +318,93 @@ impl Widget for EmailAccountSetup { } impl EmailAccountSetup { - fn read_draft(&mut self, cx: &mut Cx) -> AccountDraft { - AccountDraft { - address: self.view.text_input(cx, ids!(address_input)).text(), - smtp_server: self.view.text_input(cx, ids!(server_input)).text(), - smtp_port: self.view.text_input(cx, ids!(port_input)).text(), + /// Reflect the selected backend in the chooser and swap the form. + /// + /// The summary is the honest trade-off from `BackendKind::summary`, + /// so the security difference between the two choices is visible + /// rather than implied (C1). + fn update_chooser(&mut self, cx: &mut Cx) { + let kind = self.backend_kind; + self.view + .label(cx, ids!(chooser_selected)) + .set_text(cx, &format!("Selected: {}", kind.label())); + self.view + .label(cx, ids!(chooser_summary)) + .set_text(cx, kind.summary()); + + self.view + .view(cx, ids!(imap_form)) + .set_visible(cx, kind == BackendKind::ImapSmtp); + self.view + .view(cx, ids!(proxy_form)) + .set_visible(cx, kind == BackendKind::ProxyApi); + self.view.redraw(cx); + } + + /// Fill IMAP + SMTP servers from the address, only over blank fields + /// (or fields we filled ourselves). Reuses the domain's provider + /// table so the Outlook/Proton special cases stay in one place. + fn autofill_provider(&mut self, cx: &mut Cx) { + let address = self.view.text_input(cx, ids!(address_input)).text(); + + let draft = BackendDraft { + kind: BackendKind::ImapSmtp, + imap_server: self.view.text_input(cx, ids!(imap_server_input)).text(), + imap_port: self.view.text_input(cx, ids!(imap_port_input)).text(), + smtp_server: self.view.text_input(cx, ids!(smtp_server_input)).text(), + smtp_port: self.view.text_input(cx, ids!(smtp_port_input)).text(), username: self.view.text_input(cx, ids!(username_input)).text(), - password: self.view.text_input(cx, ids!(password_input)).text(), + ..Default::default() + }; + + let can_autofill = self.autofilled_server + || (draft.imap_server.trim().is_empty() && draft.smtp_server.trim().is_empty()); + if !can_autofill { + return; + } + + let filled = draft.with_provider_defaults(&address); + // with_provider_defaults only fills fields that were blank, so a + // filled draft with empty servers means the domain is unknown. + if filled.smtp_server.is_empty() { + return; + } + + self.view + .text_input(cx, ids!(smtp_server_input)) + .set_text(cx, &filled.smtp_server); + self.view + .text_input(cx, ids!(smtp_port_input)) + .set_text(cx, &filled.smtp_port); + self.view + .text_input(cx, ids!(imap_server_input)) + .set_text(cx, &filled.imap_server); + self.view + .text_input(cx, ids!(imap_port_input)) + .set_text(cx, &filled.imap_port); + self.view + .text_input(cx, ids!(username_input)) + .set_text(cx, &filled.username); + self.autofilled_server = true; + self.view.redraw(cx); + } + + fn read_draft(&mut self, cx: &mut Cx) -> SetupDraft { + SetupDraft { + address: self.view.text_input(cx, ids!(address_input)).text(), display_name: self.view.text_input(cx, ids!(display_name_input)).text(), + backend: BackendDraft { + kind: self.backend_kind, + imap_server: self.view.text_input(cx, ids!(imap_server_input)).text(), + imap_port: self.view.text_input(cx, ids!(imap_port_input)).text(), + smtp_server: self.view.text_input(cx, ids!(smtp_server_input)).text(), + smtp_port: self.view.text_input(cx, ids!(smtp_port_input)).text(), + username: self.view.text_input(cx, ids!(username_input)).text(), + password: self.view.text_input(cx, ids!(password_input)).text(), + api_url: self.view.text_input(cx, ids!(api_url_input)).text(), + api_token: self.view.text_input(cx, ids!(api_token_input)).text(), + label: self.view.text_input(cx, ids!(label_input)).text(), + }, } } @@ -225,23 +417,47 @@ impl EmailAccountSetup { } /// Prefill from a previously-entered account after a failure, so the - /// user fixes one field instead of retyping six. + /// user fixes one field instead of retyping the lot. The secret is + /// deliberately not restored -- it is session-only. pub fn prefill(&mut self, cx: &mut Cx, account: &nigig_core::email_account::EmailAccount) { self.view .text_input(cx, ids!(address_input)) .set_text(cx, &account.address); - self.view - .text_input(cx, ids!(server_input)) - .set_text(cx, &account.smtp_server); - self.view - .text_input(cx, ids!(port_input)) - .set_text(cx, &account.smtp_port.to_string()); - self.view - .text_input(cx, ids!(username_input)) - .set_text(cx, &account.username); self.view .text_input(cx, ids!(display_name_input)) .set_text(cx, &account.display_name); + + use nigig_core::mail_backend::BackendSettings; + self.backend_kind = account.backend.kind(); + match &account.backend { + BackendSettings::ImapSmtp(s) => { + self.view + .text_input(cx, ids!(imap_server_input)) + .set_text(cx, &s.imap_server); + self.view + .text_input(cx, ids!(imap_port_input)) + .set_text(cx, &s.imap_port.to_string()); + self.view + .text_input(cx, ids!(smtp_server_input)) + .set_text(cx, &account.smtp_server); + self.view + .text_input(cx, ids!(smtp_port_input)) + .set_text(cx, &account.smtp_port.to_string()); + self.view + .text_input(cx, ids!(username_input)) + .set_text(cx, &account.username); + } + BackendSettings::ProxyApi(s) => { + self.view + .text_input(cx, ids!(api_url_input)) + .set_text(cx, &s.api_url); + self.view + .text_input(cx, ids!(label_input)) + .set_text(cx, &s.label); + } + } + + self.update_chooser(cx); self.view.redraw(cx); } } diff --git a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs index 322aae0..d225c9e 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs @@ -20,12 +20,15 @@ // with no path to any mail, and the credentials form was on the Bulk tab. use makepad_widgets::*; -use nigig_core::email_account::{AccountDraft, EmailAccount, SessionState}; +use nigig_core::email_account::{EmailAccount, SessionState}; use nigig_core::email_store::{ group_by_sender, sample_thread, thread_for_sender, total_unread, EmailMessage, EmailThreadSummary, }; -use nigig_core::email_worker::{spawn_smtp_test, EmailWorkerAction, SmtpConfig}; +use nigig_core::email_worker::{ + spawn_proxy_verify, spawn_smtp_test, EmailWorkerAction, SmtpConfig, +}; +use nigig_core::mail_backend::{BackendSettings, SetupDraft}; use nigig_core::secret::Secret; use nigig_uikit::shared::conversation::conversation_preview::{ SharedConversationPreviewAction, SharedConversationPreviewProps, @@ -211,16 +214,21 @@ impl Widget for EmailInboxPage { // Setup form asked to connect. if let EmailAccountSetupAction::Connect(draft) = action.as_widget_action().cast() { - self.begin_connect(cx, draft); + self.begin_connect(cx, *draft); } } - // SMTP test result decides signed-in vs failed. + // Connection test result (SMTP or proxy) decides signed-in vs + // failed. Both carry a plain `Result<(), String>`, so the flow is + // transport-agnostic. if let Event::Actions(actions) = event { for action in actions { if let Some(EmailWorkerAction::SmtpTestResult(result)) = action.downcast_ref() { self.finish_connect(cx, result.clone()); } + if let Some(EmailWorkerAction::ProxyVerifyResult(result)) = action.downcast_ref() { + self.finish_connect(cx, result.clone()); + } } } @@ -302,9 +310,10 @@ fn format_thread_time(ms: i64) -> String { } impl EmailInboxPage { - /// Validate the draft and start an SMTP connection test. - fn begin_connect(&mut self, cx: &mut Cx, draft: AccountDraft) { - let (account, password) = match draft.validate() { + /// Validate the setup draft and start the connection test for the + /// chosen backend. + fn begin_connect(&mut self, cx: &mut Cx, draft: SetupDraft) { + let validated = match draft.validate() { Ok(v) => v, Err(errors) => { let msg = errors @@ -317,36 +326,49 @@ impl EmailInboxPage { } }; + let account = validated.account.clone(); self.session = SessionState::Verifying; self.pending = Some(account.clone()); - self.password = password.clone(); + self.password = validated.secret.clone(); - // A4 follow-up: warn on a from/username mismatch before spending a - // round trip. Not an error -- some providers allow send-as aliases - // -- but it is the commonest cause of a silent rejection, so the - // user should see it while they can still fix it. - let probe = SmtpConfig { - server: account.smtp_server.clone(), - port: account.smtp_port, - username: account.username.clone(), - password: password.clone(), - from: account.address.clone(), - }; - if let Some(warning) = nigig_core::email_worker::config_warning(&probe) { - self.setup_error(cx, &warning); + match validated.backend { + BackendSettings::ImapSmtp(_) => { + // A4 follow-up: warn on a from/username mismatch before + // spending a round trip. Not an error -- some providers + // allow send-as aliases -- but it is the commonest cause + // of a silent rejection, so the user should see it while + // they can still fix it. + let probe = SmtpConfig { + server: account.smtp_server.clone(), + port: account.smtp_port, + username: account.username.clone(), + password: validated.secret.clone(), + from: account.address.clone(), + }; + if let Some(warning) = nigig_core::email_worker::config_warning(&probe) { + self.setup_error(cx, &warning); + } + + // Reuse the existing worker. A successful SMTP handshake + // with AUTH is the only credential check available without + // an IMAP client, and it is the honest one: it proves the + // account can send, which is what this app can currently + // do with it. + spawn_smtp_test(SmtpConfig { + server: account.smtp_server.clone(), + port: account.smtp_port, + username: account.username.clone(), + password: validated.secret, + from: account.address.clone(), + }); + } + BackendSettings::ProxyApi(settings) => { + // C1d: verify the token against the mail service instead. + // No SMTP round trip, and the same "checking connection" + // state the direct backend uses. + spawn_proxy_verify(settings, validated.secret); + } } - - // Reuse the existing worker. A successful SMTP handshake with - // AUTH is the only credential check available without an IMAP - // client, and it is the honest one: it proves the account can - // send, which is what this app can currently do with it. - spawn_smtp_test(SmtpConfig { - server: account.smtp_server.clone(), - port: account.smtp_port, - username: account.username.clone(), - password, - from: account.address.clone(), - }); } fn finish_connect(&mut self, cx: &mut Cx, result: Result<(), String>) { diff --git a/crates/nigig-core/src/email_worker.rs b/crates/nigig-core/src/email_worker.rs index e922885..ea425a0 100644 --- a/crates/nigig-core/src/email_worker.rs +++ b/crates/nigig-core/src/email_worker.rs @@ -102,10 +102,34 @@ pub fn spawn_smtp_test(config: SmtpConfig) { }); } +/// Spawn a proxy-backend verification against the mail service (C1d). +/// +/// The proxy equivalent of `spawn_smtp_test`: prove the token is accepted +/// before the account is saved. Uses the native `reqwest` transport or the +/// wasm `fetch` transport, selected at compile time, and posts +/// `EmailWorkerAction::ProxyVerifyResult` either way. +pub fn spawn_proxy_verify(settings: crate::mail_backend::ProxySettings, token: Secret) { + crate::platform::spawn(async move { + let client = crate::mail_proxy::ProxyApiClient::new(settings, token); + + #[cfg(not(target_arch = "wasm32"))] + let result = client + .verify(&crate::mail_proxy::ReqwestTransport::default()) + .await; + #[cfg(target_arch = "wasm32")] + let result = client.verify(&crate::mail_proxy::WasmFetchTransport).await; + + // Map the structured error to its user-facing message so the UI + // shows the same text it would for a failed SMTP test. + Cx::post_action(EmailWorkerAction::ProxyVerifyResult( + result.map_err(|e| e.message()), + )); + }); +} + /// Shown when a send is attempted without enough settings to try. pub const INCOMPLETE_CONFIG_MESSAGE: &str = "Missing account settings. Check the server, port, username, password and from address."; - /// RFC 5322 caps a header line at 998 octets. A subject longer than that is /// silently folded or truncated by the provider, so reject it here where we /// can say why. @@ -513,6 +537,10 @@ async fn call_email_api(json_body: &str) -> Result<(), String> { #[derive(Clone, Debug)] pub enum EmailWorkerAction { SmtpTestResult(Result<(), String>), + /// Result of verifying a proxy-backend account against the mail + /// service (C1d). Same `Result<(), String>` shape as the SMTP test so + /// the UI's "checking connection" flow is transport-agnostic. + ProxyVerifyResult(Result<(), String>), SendResult(Result<(), String>), None, } @@ -590,6 +618,39 @@ mod tests { ); } + // ---- A2/A3: the transport policy, constructed -------------------- + + /// `build_transport` is the network-adjacent code that IS testable: + /// it assembles a lettre transport (`relay` for implicit TLS, + /// `starttls_relay` for everything else) without opening a socket. + /// This pins that both arms construct, so the A2/A3 policy -- never + /// cleartext, relay on 465, mandatory STARTTLS elsewhere -- cannot be + /// broken by a refactor that merely compiles. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn build_transport_constructs_for_every_port() { + // lettre's transport owns a connection pool whose drop touches the + // tokio runtime, so construction and drop happen inside one. + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime"); + rt.block_on(async { + let creds = lettre::transport::smtp::authentication::Credentials::new( + "jane@example.com".into(), + "hunter2".into(), + ); + // 465 is implicit TLS; 587, 25 and anything else are STARTTLS. + for port in [IMPLICIT_TLS_PORT, 587u16, 25, 2525, 8465] { + let cfg = SmtpConfig { port, ..good() }; + assert!( + build_transport(&cfg, creds.clone()).is_ok(), + "port {port} should construct a transport" + ); + } + }); + } + // ---- A4: local validation before spending a round trip ------------ #[test] diff --git a/crates/nigig-core/src/lib.rs b/crates/nigig-core/src/lib.rs index 9a09cc4..234c2ef 100644 --- a/crates/nigig-core/src/lib.rs +++ b/crates/nigig-core/src/lib.rs @@ -23,6 +23,7 @@ pub mod secret; pub mod email_send; pub mod email_store; pub mod mail_backend; +pub mod mail_proxy; pub mod email_worker; pub use dir::app_data_dir; diff --git a/crates/nigig-core/src/mail_backend.rs b/crates/nigig-core/src/mail_backend.rs index 3e383f2..4d836b3 100644 --- a/crates/nigig-core/src/mail_backend.rs +++ b/crates/nigig-core/src/mail_backend.rs @@ -349,6 +349,124 @@ impl BackendDraft { } } +/// A filled-in setup form: account identity plus a backend section. +/// +/// C1c. The chooser makes the setup form two forms in one, so the draft +/// the UI hands over must carry the identity (address, display name) that +/// every backend needs AND the backend-specific fields. Keeping this here +/// rather than in the widget is the same call as `AccountDraft`: the +/// widget only moves strings, and the branching lives where it is tested. +#[derive(Clone, Debug, Default)] +pub struct SetupDraft { + pub address: String, + pub display_name: String, + pub backend: BackendDraft, +} + +/// Why a setup draft was rejected. One arm per source, so the form can +/// render every problem in one pass and the message text stays with the +/// error type that owns it. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SetupError { + Account(crate::email_account::AccountError), + Backend(BackendError), +} + +impl SetupError { + pub fn message(&self) -> String { + match self { + SetupError::Account(e) => e.message().to_string(), + SetupError::Backend(e) => e.message(), + } + } +} + +/// A setup draft that passed: the persistable account, the persistable +/// backend settings, and the session-only secret. +#[derive(Clone, Debug)] +pub struct ValidatedSetup { + pub account: crate::email_account::EmailAccount, + pub backend: BackendSettings, + pub secret: Secret, +} + +impl SetupDraft { + /// Validate identity + backend together, reporting every problem. + /// + /// The identity (address) is checked here because both backends need + /// it. The backend section is delegated to `BackendDraft::validate`, + /// and for the direct backend the SMTP send fields are checked too — + /// a direct backend with no usable SMTP server can send nothing, and + /// `BackendDraft::validate` deliberately does not know about sending. + pub fn validate(&self) -> Result> { + let mut errors = Vec::new(); + + let address = self.address.trim(); + if address.is_empty() { + errors.push(SetupError::Account(AccountError::AddressMissing)); + } else if !looks_like_email(address) { + errors.push(SetupError::Account(AccountError::AddressMalformed)); + } + + // Delegate the backend half; its errors are mapped into the + // combined list so the form shows identity and backend problems + // together. + let (settings, secret) = match self.backend.validate() { + Ok(v) => v, + Err(backend_errors) => { + errors.extend(backend_errors.into_iter().map(SetupError::Backend)); + // No settings to proceed with; the identity errors, if + // any, are already collected above. + return Err(errors); + } + }; + + // The direct backend sends over SMTP, so its send fields are + // validated here (the same rules AccountDraft uses) rather than + // duplicated into BackendDraft. + let (smtp_server, smtp_port, username) = match &settings { + BackendSettings::ImapSmtp(s) => { + let server = s.smtp_server.trim(); + if server.is_empty() { + errors.push(SetupError::Account(AccountError::ServerMissing)); + } else if !crate::email_account::looks_like_hostname(server) { + errors.push(SetupError::Account(AccountError::ServerMalformed)); + } + if s.smtp_port == 0 { + errors.push(SetupError::Account(AccountError::PortInvalid)); + } + let username = if s.username.trim().is_empty() { + address.to_string() + } else { + s.username.trim().to_string() + }; + (server.to_string(), s.smtp_port, username) + } + // The proxy backend sends through the service, so the legacy + // SMTP fields are meaningless and left unset rather than + // carrying a half-truth. + BackendSettings::ProxyApi(_) => (String::new(), 0u16, address.to_string()), + }; + + if !errors.is_empty() { + return Err(errors); + } + + Ok(ValidatedSetup { + account: crate::email_account::EmailAccount { + address: address.to_string(), + smtp_server, + smtp_port, + username, + display_name: self.display_name.trim().to_string(), + backend: settings.clone(), + }, + backend: settings, + secret, + }) + } +} + /// What every backend must be able to do. /// /// Deliberately small. Anything that can be computed above this line @@ -446,6 +564,7 @@ pub fn backend_from(settings: BackendSettings, secret: Secret) -> Box BackendDraft { BackendDraft { @@ -705,6 +824,20 @@ mod tests { assert_eq!(d.smtp_server, "my.own.smtp"); } + /// Proton has no public IMAP: mail is read through the local Proton + /// Bridge. Defaulting to a remote `imap.protonmail.ch` would point the + /// user at a host that does not resolve. + #[test] + fn proton_defaults_to_the_local_bridge_not_a_remote_imap_host() { + let d = BackendDraft { + kind: BackendKind::ImapSmtp, + ..Default::default() + } + .with_provider_defaults("jane@protonmail.com"); + assert_eq!(d.smtp_server, "smtp.protonmail.ch"); + assert_eq!(d.imap_server, "127.0.0.1"); + } + #[test] fn provider_defaults_do_nothing_for_an_unknown_or_invalid_address() { let d = BackendDraft::default().with_provider_defaults("jane@my-company.co.ke"); @@ -826,4 +959,133 @@ mod tests { // The insecure-URL message must explain WHY, not just refuse. assert!(BackendError::ApiUrlInsecure.message().contains("clear")); } + + // ---- SetupDraft (C1c) ---------------------------------------------- + + fn setup_draft(kind: BackendKind) -> SetupDraft { + SetupDraft { + address: "jane@example.com".into(), + display_name: "Jane".into(), + backend: match kind { + BackendKind::ImapSmtp => imap_draft(), + BackendKind::ProxyApi => proxy_draft(), + }, + } + } + + /// A valid direct-backend setup yields an account whose legacy SMTP + /// fields mirror the backend settings, so the existing send path keeps + /// working, plus the imap settings and the (split-out) secret. + #[test] + fn a_valid_direct_setup_validates_into_account_backend_and_secret() { + let v = setup_draft(BackendKind::ImapSmtp).validate().unwrap(); + assert_eq!(v.account.address, "jane@example.com"); + assert_eq!(v.account.smtp_server, "smtp.example.com"); + assert_eq!(v.account.smtp_port, 587); + assert_eq!(v.account.username, "jane@example.com"); + assert_eq!(v.secret.expose(), "hunter2"); + match &v.backend { + BackendSettings::ImapSmtp(s) => { + assert_eq!(s.imap_server, "imap.example.com"); + assert_eq!(s.imap_port, 993); + } + other => panic!("wrong variant: {other:?}"), + } + // The account's backend section must be the real settings, not + // the default (which is also ImapSmtp but with empty fields). + assert_eq!(v.account.backend, v.backend); + } + + /// A valid proxy setup carries no SMTP fields — they are meaningless + /// when sending goes through the service. + #[test] + fn a_valid_proxy_setup_leaves_smtp_fields_unset() { + let v = setup_draft(BackendKind::ProxyApi).validate().unwrap(); + assert_eq!(v.account.address, "jane@example.com"); + assert!(v.account.smtp_server.is_empty()); + assert_eq!(v.account.smtp_port, 0); + assert_eq!(v.secret.expose(), "tok_abc123"); + match &v.backend { + BackendSettings::ProxyApi(s) => assert_eq!(s.api_url, "https://mail.example.com/api"), + other => panic!("wrong variant: {other:?}"), + } + } + + #[test] + fn an_identity_and_backend_problem_are_reported_together() { + let d = SetupDraft { + address: "not-an-email".into(), + backend: BackendDraft { + kind: BackendKind::ProxyApi, + api_url: "http://insecure.example.com".into(), + api_token: String::new(), + ..Default::default() + }, + ..Default::default() + }; + let errors = d.validate().unwrap_err(); + assert!(errors.iter().any(|e| matches!(e, SetupError::Account(_)))); + assert!(errors + .iter() + .any(|e| matches!(e, SetupError::Backend(BackendError::ApiUrlInsecure)))); + assert!(errors + .iter() + .any(|e| matches!(e, SetupError::Backend(BackendError::TokenMissing)))); + } + + /// The direct backend must be able to send, so a missing or malformed + /// SMTP server is an error even though BackendDraft alone does not + /// check it. + #[test] + fn a_direct_setup_without_a_usable_smtp_server_is_refused() { + let d = SetupDraft { + backend: BackendDraft { + smtp_server: String::new(), + ..imap_draft() + }, + ..setup_draft(BackendKind::ImapSmtp) + }; + let errors = d.validate().unwrap_err(); + assert!(errors + .iter() + .any(|e| matches!(e, SetupError::Account(AccountError::ServerMissing)))); + + let d = SetupDraft { + backend: BackendDraft { + smtp_server: "https://smtp.example.com".into(), + ..imap_draft() + }, + ..setup_draft(BackendKind::ImapSmtp) + }; + let errors = d.validate().unwrap_err(); + assert!(errors + .iter() + .any(|e| matches!(e, SetupError::Account(AccountError::ServerMalformed)))); + } + + /// The username falls back to the address for the direct backend, the + /// same rule AccountDraft applies. + #[test] + fn a_direct_setup_defaults_username_to_the_address() { + let d = SetupDraft { + backend: BackendDraft { + username: String::new(), + ..imap_draft() + }, + ..setup_draft(BackendKind::ImapSmtp) + }; + let v = d.validate().unwrap(); + assert_eq!(v.account.username, "jane@example.com"); + } + + #[test] + fn every_setup_error_delegates_to_an_actionable_message() { + let all = [ + SetupError::Account(AccountError::AddressMissing), + SetupError::Backend(BackendError::ApiUrlInsecure), + ]; + for e in &all { + assert!(e.message().len() > 10, "{e:?} message too terse"); + } + } } diff --git a/crates/nigig-core/src/mail_proxy.rs b/crates/nigig-core/src/mail_proxy.rs new file mode 100644 index 0000000..aafc421 --- /dev/null +++ b/crates/nigig-core/src/mail_proxy.rs @@ -0,0 +1,745 @@ +//! The proxy backend's HTTP client (Phase C1d). +//! +//! C1 decided to support two backends, user-selectable. `mail_backend.rs` +//! built the trait boundary and both *validation* halves (C1a/C1b); this +//! module is the first backend's *network* half: a thin HTTP client that +//! talks to the Nigig mail service. +//! +//! ## Why this one first +//! +//! The plan says to implement the proxy before IMAP, and for a reason +//! that is not just about size: a browser cannot open a raw TCP socket, +//! so IMAP-on-device can never work on wasm. A proxy always had to exist +//! for the browser target, which makes this the backend that exercises +//! the `MailBackend` seam end to end on every platform. +//! +//! ## What is testable here, and what is not +//! +//! The bugs live in the *edges of the wire*: what method/path a request +//! takes, whether the token goes in an `Authorization` header and nowhere +//! else, what JSON goes out, and how a non-200 status or a malformed body +//! becomes a message the user can act on. All of that is pure and is +//! tested below against a mock transport -- no server required. +//! +//! The actual socket is deliberately thin and untested, exactly like the +//! SMTP transport: `reqwest` on native, `fetch` on wasm. See THREAT_MODEL +//! T-E4 for why the proxy path does not go through `lettre` (and therefore +//! why recipient/header validation happens in `email_send`, not here). + +use crate::email_send::EmailSendRequest; +use crate::email_store::EmailMessage; +use crate::secret::Secret; +use serde::{Deserialize, Serialize}; + +/// The payload of a `POST /send`, minus the credential. +/// +/// Recipients are carried as mailbox strings (`Name ` or bare +/// `addr`), because that is what `Recipient::to_mailbox_string()` produces +/// and what the server can hand to a relay unchanged. Structured JSON here +/// would just be re-parsed on the other side. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct SendPayload { + pub recipients: Vec, + pub subject: String, + pub body: String, +} + +/// A single wire request to the proxy API, minus the credential. +/// +/// The token is NOT a field here: it is passed to the transport +/// separately (as a `Secret`), so this type can be `Debug`-printed in +/// logs and tests without leaking anything. The only place the token +/// becomes a header is inside the transport's `execute`. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ProxyRequest { + /// `GET /inbox` -- list the account's messages. + ListInbox, + /// `POST /verify` -- prove the token is accepted. + Verify, + /// `POST /send` -- deliver one message. + Send(SendPayload), +} + +impl ProxyRequest { + pub fn method(&self) -> &'static str { + match self { + ProxyRequest::ListInbox => "GET", + ProxyRequest::Verify | ProxyRequest::Send(_) => "POST", + } + } + + pub fn path(&self) -> &'static str { + match self { + ProxyRequest::ListInbox => "/inbox", + ProxyRequest::Verify => "/verify", + ProxyRequest::Send(_) => "/send", + } + } + + /// Serialised JSON body, if any. `Send` is the only request with one. + pub fn json_body(&self) -> Option { + match self { + ProxyRequest::Send(payload) => Some(serde_json::to_string(payload).ok()?), + _ => None, + } + } +} + +/// The outcome of one HTTP exchange, ready for the pure parsers below. +/// +/// Status + body, nothing else. Keeping the response this small is what +/// lets every parse path be exercised with a mock transport. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProxyResponse { + pub status: u16, + pub body: String, +} + +/// Why a proxy operation failed. +/// +/// One variant per thing the user can act on, mirroring `BackendError` / +/// `SendError`. The transport's own error string is carried verbatim +/// rather than mapped, because it is usually the underlying library's +/// wording and there is nothing to add. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ProxyError { + /// The transport could not complete the exchange (DNS, TLS, timeout). + Transport(String), + /// 401/403 -- the token was rejected or revoked. Actionable: re-enter + /// it, and nothing about the account settings is wrong. + Unauthorized, + /// Any other non-2xx status. The proxy is up but refused. + Server(u16), + /// A 2xx response whose body does not parse. The wire contract moved. + MalformedResponse(String), + /// The server answered a `send` with a specific refusal (4xx with an + /// `error` field), e.g. a recipient it will not relay to. + Rejected(String), +} + +impl ProxyError { + pub fn message(&self) -> String { + match self { + ProxyError::Transport(e) => format!("Could not reach the mail service: {e}"), + ProxyError::Unauthorized => { + "The mail service rejected your token. Check it and try again.".into() + } + ProxyError::Server(status) => { + format!("The mail service returned an error ({status}). Try again shortly.") + } + ProxyError::MalformedResponse(_) => "The mail service sent an unexpected reply.".into(), + ProxyError::Rejected(why) => format!("The mail service refused to send: {why}"), + } + } +} + +/// The thing that actually speaks HTTP. Everything else in this module is +/// pure; this trait is the only place a socket is opened, and it is the +/// only place the token is turned into a header. +/// +/// `async fn` in a trait, not `dyn`-friendly -- each caller is generic +/// over the transport, so there is no boxing and tests inject a mock +/// struct that never touches the network. +// The trait is public because the generic client methods are, but it is +// only implemented inside this crate (reqwest/fetch) and by tests. The +// future's auto-traits are resolved at the monomorphised call sites, which +// is fine here, so the `async_fn_in_trait` lint is noise for this use. +#[allow(async_fn_in_trait)] +pub trait ProxyTransport { + async fn execute( + &self, + base: &str, + token: &Secret, + request: &ProxyRequest, + ) -> Result; +} + +/// A configured proxy backend client. +/// +/// Holds the non-secret settings and the session token. All three +/// operations are generic over the transport so the same logic is proven +/// against a mock in tests and driven by `reqwest`/`fetch` in production. +#[derive(Clone, Debug)] +pub struct ProxyApiClient { + pub settings: crate::mail_backend::ProxySettings, + pub token: Secret, +} + +impl ProxyApiClient { + pub fn new(settings: crate::mail_backend::ProxySettings, token: Secret) -> Self { + Self { settings, token } + } + + /// Prove the token is accepted before saving the account (C1d verify). + pub async fn verify(&self, t: &T) -> Result<(), ProxyError> { + let req = ProxyRequest::Verify; + let resp = t + .execute(&self.settings.api_url, &self.token, &req) + .await + .map_err(ProxyError::Transport)?; + parse_verify_response(resp.status, &resp.body) + } + + /// Fetch the account's inbox as a flat `Vec`. + /// + /// Everything above the `MailBackend` seam -- grouping, previews, the + /// inbox list -- consumes this shape and does not care where it came + /// from, which is what made two backends a contained change (C2). + pub async fn list_inbox( + &self, + t: &T, + ) -> Result, ProxyError> { + let req = ProxyRequest::ListInbox; + let resp = t + .execute(&self.settings.api_url, &self.token, &req) + .await + .map_err(ProxyError::Transport)?; + parse_inbox_response(resp.status, &resp.body) + } + + /// Deliver one message through the service. + pub async fn send( + &self, + t: &T, + request: &EmailSendRequest, + ) -> Result<(), ProxyError> { + let payload = SendPayload { + recipients: request + .recipients + .iter() + .map(|r| r.to_mailbox_string()) + .collect(), + subject: request.subject.clone(), + body: request.body.clone(), + }; + let req = ProxyRequest::Send(payload); + let resp = t + .execute(&self.settings.api_url, &self.token, &req) + .await + .map_err(ProxyError::Transport)?; + parse_send_response(resp.status, &resp.body) + } +} + +/// The inbox wire shape. +#[derive(Deserialize)] +struct InboxResponse { + messages: Vec, +} + +/// The error wire shape, for a `send` refusal. +#[derive(Deserialize)] +struct ApiErrorBody { + #[serde(default)] + error: String, +} + +/// Map a status + body onto the inbox parse. +/// +/// Pure, so every branch -- 401, 500, malformed 200 -- is a test, not a +/// prayer. Status is checked before the body is parsed: a 401 with an +/// HTML login page must not surface a confusing JSON error. +pub fn parse_inbox_response(status: u16, body: &str) -> Result, ProxyError> { + if status == 401 || status == 403 { + return Err(ProxyError::Unauthorized); + } + if !(200..300).contains(&status) { + return Err(ProxyError::Server(status)); + } + let parsed: InboxResponse = + serde_json::from_str(body).map_err(|e| ProxyError::MalformedResponse(e.to_string()))?; + Ok(parsed.messages) +} + +/// `POST /verify` only needs to succeed; the body is ignored on 2xx. +pub fn parse_verify_response(status: u16, body: &str) -> Result<(), ProxyError> { + if status == 401 || status == 403 { + return Err(ProxyError::Unauthorized); + } + if !(200..300).contains(&status) { + return Err(ProxyError::Server(status)); + } + let _ = body; + Ok(()) +} + +/// `POST /send` success is a bare 2xx; a 4xx with an `error` field is a +/// specific, user-readable refusal rather than a generic server error. +pub fn parse_send_response(status: u16, body: &str) -> Result<(), ProxyError> { + if status == 401 || status == 403 { + return Err(ProxyError::Unauthorized); + } + if (200..300).contains(&status) { + return Ok(()); + } + if let Ok(e) = serde_json::from_str::(body) { + if !e.error.trim().is_empty() { + return Err(ProxyError::Rejected(e.error)); + } + } + Err(ProxyError::Server(status)) +} + +// --------------------------------------------------------------------- +// Native transport: reqwest. +// --------------------------------------------------------------------- + +#[cfg(not(target_arch = "wasm32"))] +pub struct ReqwestTransport { + client: reqwest::Client, +} + +#[cfg(not(target_arch = "wasm32"))] +impl Default for ReqwestTransport { + fn default() -> Self { + Self { + client: reqwest::Client::new(), + } + } +} + +#[cfg(not(target_arch = "wasm32"))] +impl ProxyTransport for ReqwestTransport { + async fn execute( + &self, + base: &str, + token: &Secret, + request: &ProxyRequest, + ) -> Result { + let url = format!("{}{}", base.trim_end_matches('/'), request.path()); + let mut builder = match request.method() { + "GET" => self.client.get(&url), + _ => self.client.post(&url), + }; + // The one place the token becomes a header. + builder = builder.bearer_auth(token.expose()); + if let Some(body) = request.json_body() { + builder = builder + .header("Content-Type", "application/json") + .body(body); + } + let resp = builder.send().await.map_err(|e| e.to_string())?; + let status = resp.status().as_u16(); + let body = resp.text().await.map_err(|e| e.to_string())?; + Ok(ProxyResponse { status, body }) + } +} + +// --------------------------------------------------------------------- +// Wasm transport: fetch. Kept here so the crate still compiles on wasm, +// where reqwest and tokio do not exist. +// --------------------------------------------------------------------- + +#[cfg(target_arch = "wasm32")] +pub struct WasmFetchTransport; + +#[cfg(target_arch = "wasm32")] +impl ProxyTransport for WasmFetchTransport { + async fn execute( + &self, + base: &str, + token: &Secret, + request: &ProxyRequest, + ) -> Result { + use wasm_bindgen::JsCast; + use wasm_bindgen::JsValue; + use wasm_bindgen_futures::JsFuture; + + let url = format!("{}{}", base.trim_end_matches('/'), request.path()); + let mut opts = web_sys::RequestInit::new(); + opts.method(request.method()); + if let Some(body) = request.json_body() { + opts.body(Some(&JsValue::from_str(&body))); + } + let req = web_sys::Request::new_with_str_and_init(&url, &opts) + .map_err(|e| format!("Failed to build request: {e:?}"))?; + req.headers() + .set("Authorization", &format!("Bearer {}", token.expose())) + .map_err(|e| format!("Failed to set Authorization: {e:?}"))?; + if request.json_body().is_some() { + req.headers() + .set("Content-Type", "application/json") + .map_err(|e| format!("Failed to set Content-Type: {e:?}"))?; + } + let window = web_sys::window().ok_or_else(|| "No global window".to_string())?; + let resp_value = JsFuture::from(window.fetch_with_request(&req)) + .await + .map_err(|e| format!("Fetch failed: {e:?}"))?; + let resp: web_sys::Response = resp_value + .dyn_into() + .map_err(|_| "Response is not a Response".to_string())?; + let status = resp.status(); + let body = JsFuture::from( + resp.text() + .map_err(|_| "Failed to read response body".to_string())?, + ) + .await + .map_err(|e| format!("Failed to read response: {e:?}"))?; + Ok(ProxyResponse { + status, + body: body.as_string().unwrap_or_default(), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mail_backend::{BackendSettings, ProxySettings}; + use crate::secret::Secret; + + /// Poll a future to completion without a runtime. + /// + /// `futures::executor` is compiled out (nigig-core pins `futures` with + /// `default-features = false`), and a mock transport resolves on the + /// first poll anyway. This is dependency-free: the mock's future is + /// ready immediately, so the loop returns after one poll. + fn block_on(fut: F) -> F::Output { + use std::sync::Arc; + use std::task::{Context, Poll, Wake, Waker}; + + struct NoopWaker; + impl Wake for NoopWaker { + fn wake(self: Arc) {} + } + + let waker = Waker::from(Arc::new(NoopWaker)); + let mut cx = Context::from_waker(&waker); + let mut fut = Box::pin(fut); + loop { + match fut.as_mut().poll(&mut cx) { + Poll::Ready(v) => return v, + Poll::Pending => std::thread::yield_now(), + } + } + } + + fn client() -> ProxyApiClient { + ProxyApiClient::new( + ProxySettings { + api_url: "https://mail.example.com/api".into(), + label: "Work".into(), + }, + Secret::new("tok_abc123"), + ) + } + + /// A transport that records what it was handed and returns a canned + /// response, so the glue -- method, path, token header, parsing -- is + /// exercised without a socket. + struct MockTransport { + status: u16, + body: String, + saw_method: std::cell::RefCell, + saw_path: std::cell::RefCell, + saw_token: std::cell::RefCell, + saw_body: std::cell::RefCell>, + } + + impl MockTransport { + fn new(status: u16, body: &str) -> Self { + Self { + status, + body: body.to_string(), + saw_method: Default::default(), + saw_path: Default::default(), + saw_token: Default::default(), + saw_body: Default::default(), + } + } + } + + impl ProxyTransport for MockTransport { + async fn execute( + &self, + _base: &str, + token: &Secret, + request: &ProxyRequest, + ) -> Result { + *self.saw_method.borrow_mut() = request.method().to_string(); + *self.saw_path.borrow_mut() = request.path().to_string(); + *self.saw_token.borrow_mut() = token.expose().to_string(); + *self.saw_body.borrow_mut() = request.json_body(); + Ok(ProxyResponse { + status: self.status, + body: self.body.clone(), + }) + } + } + + fn one_message() -> EmailMessage { + EmailMessage { + id: "m1".into(), + from_address: "alerts@bank.co.ke".into(), + from_name: "Equity Alerts".into(), + subject: "Statement ready".into(), + body: "Your statement is ready.".into(), + date_ms: 1_767_225_600_000, + is_read: false, + is_outgoing: false, + } + } + + // ---- request construction ----------------------------------------- + + #[test] + fn inbox_is_a_get_to_inbox_with_no_body() { + let req = ProxyRequest::ListInbox; + assert_eq!(req.method(), "GET"); + assert_eq!(req.path(), "/inbox"); + assert_eq!(req.json_body(), None); + } + + #[test] + fn verify_and_send_are_posts() { + assert_eq!(ProxyRequest::Verify.method(), "POST"); + assert_eq!(ProxyRequest::Verify.path(), "/verify"); + assert_eq!(ProxyRequest::Verify.json_body(), None); + + let send = ProxyRequest::Send(SendPayload { + recipients: vec!["a@b.com".into()], + subject: "Hi".into(), + body: "Body".into(), + }); + assert_eq!(send.method(), "POST"); + assert_eq!(send.path(), "/send"); + assert!(send.json_body().is_some()); + } + + /// The token must not be a field of the request: a Debug print of a + /// request (in logs, in a panic, in a test failure) must not carry it. + #[test] + fn the_request_type_carries_no_secret() { + let send = ProxyRequest::Send(SendPayload { + recipients: vec!["a@b.com".into()], + subject: "Hi".into(), + body: "Body".into(), + }); + let rendered = format!("{send:?}"); + assert!(!rendered.contains("tok_abc123")); + } + + // ---- inbox parsing ------------------------------------------------- + + #[test] + fn a_200_inbox_parses_into_messages() { + let body = serde_json::json!({ "messages": [one_message()] }).to_string(); + let msgs = parse_inbox_response(200, &body).unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].from_address, "alerts@bank.co.ke"); + } + + #[test] + fn an_empty_inbox_is_zero_messages_not_an_error() { + let body = r#"{"messages":[]}"#; + assert_eq!(parse_inbox_response(200, body).unwrap().len(), 0); + } + + #[test] + fn a_401_is_reported_as_unauthorized() { + assert_eq!( + parse_inbox_response(401, "login"), + Err(ProxyError::Unauthorized) + ); + assert_eq!(parse_inbox_response(403, ""), Err(ProxyError::Unauthorized)); + } + + #[test] + fn a_server_error_carries_its_status() { + assert_eq!(parse_inbox_response(500, ""), Err(ProxyError::Server(500))); + assert_eq!( + parse_inbox_response(502, "bad gateway"), + Err(ProxyError::Server(502)) + ); + } + + /// A 200 whose body is not the documented shape must be a clear error, + /// not a panic or a silently empty inbox. + #[test] + fn a_200_with_a_malformed_body_is_an_error() { + assert!(matches!( + parse_inbox_response(200, "not json"), + Err(ProxyError::MalformedResponse(_)) + )); + // Missing the `messages` field entirely. + assert!(matches!( + parse_inbox_response(200, r#"{"other":1}"#), + Err(ProxyError::MalformedResponse(_)) + )); + } + + // ---- verify -------------------------------------------------------- + + #[test] + fn verify_succeeds_on_2xx_and_rejects_on_401() { + assert_eq!(parse_verify_response(200, "{}"), Ok(())); + assert_eq!(parse_verify_response(204, ""), Ok(())); + assert_eq!( + parse_verify_response(401, ""), + Err(ProxyError::Unauthorized) + ); + assert_eq!(parse_verify_response(500, ""), Err(ProxyError::Server(500))); + } + + // ---- send ---------------------------------------------------------- + + #[test] + fn send_succeeds_on_a_bare_2xx() { + assert_eq!(parse_send_response(200, "{}"), Ok(())); + assert_eq!(parse_send_response(202, ""), Ok(())); + } + + /// A 4xx with an `error` field is the server telling the user what is + /// wrong (a refused recipient, a quota). Surface it, don't bury it. + #[test] + fn a_send_refusal_with_an_error_field_is_surfaces_the_message() { + assert_eq!( + parse_send_response(422, r#"{"error":"recipient unknown"}"#), + Err(ProxyError::Rejected("recipient unknown".into())) + ); + } + + /// A refusal without an error field falls back to a status error. + #[test] + fn a_send_refusal_without_an_error_field_is_a_server_error() { + assert_eq!(parse_send_response(429, ""), Err(ProxyError::Server(429))); + } + + // ---- the client glue, through a mock transport --------------------- + + #[test] + fn list_inbox_sends_a_get_and_parses_the_messages() { + let t = MockTransport::new( + 200, + &serde_json::json!({ "messages": [one_message()] }).to_string(), + ); + let msgs = block_on(client().list_inbox(&t)).unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(*t.saw_method.borrow(), "GET"); + assert_eq!(*t.saw_path.borrow(), "/inbox"); + assert_eq!(*t.saw_token.borrow(), "tok_abc123"); + assert_eq!(*t.saw_body.borrow(), None); + } + + #[test] + fn send_builds_a_post_with_the_mailbox_strings() { + let t = MockTransport::new(200, "{}"); + let req = crate::email_send::EmailSendRequest { + recipients: vec![ + crate::email_send::Recipient { + address: "jane@example.com".into(), + name: "Jane".into(), + }, + crate::email_send::Recipient { + address: "bob@example.com".into(), + name: String::new(), + }, + ], + subject: "Hello".into(), + body: "World".into(), + }; + block_on(client().send(&t, &req)).unwrap(); + assert_eq!(*t.saw_method.borrow(), "POST"); + assert_eq!(*t.saw_path.borrow(), "/send"); + let body = t.saw_body.borrow().clone().expect("send has a body"); + let json: serde_json::Value = serde_json::from_str(&body).unwrap(); + assert_eq!( + json["recipients"], + serde_json::json!(["Jane ", "bob@example.com"]) + ); + assert_eq!(json["subject"], "Hello"); + assert_eq!(json["body"], "World"); + // The token rides in the auth header, never in the body. + assert!(!body.contains("tok_abc123")); + } + + #[test] + fn verify_round_trips_a_token_and_a_2xx() { + let t = MockTransport::new(200, "{}"); + block_on(client().verify(&t)).unwrap(); + assert_eq!(*t.saw_path.borrow(), "/verify"); + assert_eq!(*t.saw_token.borrow(), "tok_abc123"); + } + + #[test] + fn an_unauthorized_transport_reply_surfaces_as_unauthorized() { + let t = MockTransport::new(401, ""); + let err = block_on(client().list_inbox(&t)).unwrap_err(); + assert_eq!(err, ProxyError::Unauthorized); + } + + /// A transport that fails (DNS, TLS, timeout) carries its own message + /// through, so the user sees the real cause rather than a generic one. + #[test] + fn a_transport_failure_is_carried_not_masked() { + struct Failing; + impl ProxyTransport for Failing { + async fn execute( + &self, + _base: &str, + _token: &Secret, + _request: &ProxyRequest, + ) -> Result { + Err("connection refused".into()) + } + } + let err = block_on(client().list_inbox(&Failing)).unwrap_err(); + assert_eq!(err, ProxyError::Transport("connection refused".into())); + assert!(err.message().contains("connection refused")); + } + + // ---- error messages ------------------------------------------------ + + #[test] + fn every_proxy_error_has_a_distinct_actionable_message() { + let all = [ + ProxyError::Transport("dns".into()), + ProxyError::Unauthorized, + ProxyError::Server(500), + ProxyError::MalformedResponse("x".into()), + ProxyError::Rejected("recipient unknown".into()), + ]; + let mut seen = std::collections::HashSet::new(); + for e in &all { + let m = e.message(); + assert!(m.len() > 15, "{e:?} message too terse: {m}"); + assert!(seen.insert(m.clone()), "duplicate message for {e:?}"); + } + // The unauthorized message must tell the user what to DO, which + // distinguishes a token problem from a server problem. + assert!(ProxyError::Unauthorized.message().contains("token")); + // A rejection must carry the server's reason, not replace it. + assert!(ProxyError::Rejected("recipient unknown".into()) + .message() + .contains("recipient unknown")); + } + + /// The client holds a `Secret`, so Debug-printing it must not leak the + /// token -- the S2 property, one more layer out. + #[test] + fn debug_printing_the_client_does_not_leak_the_token() { + let rendered = format!("{:?}", client()); + assert!(rendered.contains("mail.example.com"), "show non-secrets"); + assert!(!rendered.contains("tok_abc123"), "leaked: {rendered}"); + } + + /// `backend_from` produces a ProxyBackend whose settings round-trip + /// into a client with the same secret (C1d wiring into mail_backend). + #[test] + fn the_client_builds_from_a_validated_proxy_draft() { + let (settings, secret) = crate::mail_backend::BackendDraft { + kind: crate::mail_backend::BackendKind::ProxyApi, + api_url: "https://mail.example.com/api".into(), + api_token: "tok_abc123".into(), + ..Default::default() + } + .validate() + .unwrap(); + let BackendSettings::ProxyApi(proxy_settings) = settings else { + panic!("expected proxy settings"); + }; + let client = ProxyApiClient::new(proxy_settings, secret); + assert_eq!(client.token.expose(), "tok_abc123"); + assert_eq!(client.settings.api_url, "https://mail.example.com/api"); + } +} diff --git a/tools/test-email-coverage.sh b/tools/test-email-coverage.sh new file mode 100644 index 0000000..0d6933f --- /dev/null +++ b/tools/test-email-coverage.sh @@ -0,0 +1,196 @@ +#!/usr/bin/env bash +# LLVM source-coverage for the nigig-email domain, with an enforced floor. +# +# The email domain lives in nigig-core alongside a great deal of unrelated +# code, and nigig-core depends on Makepad (email_worker posts widget actions), +# so unlike the PDF engine it cannot be copied out into a standalone pure +# workspace. This runs the domain tests in place, instruments the build, and +# then reports coverage over ONLY the seven email source files -- Makepad's +# generated code and the rest of nigig-core are excluded from the count. +# +# Why a floor and not a report: a number that is only printed drifts down. +# The whole point of Phase A-E was turning "the SMTP password serialises" and +# "the bulk tab cannot bulk-send" from findings into tests; a floor keeps +# that from silently reverting. Same reasoning as tools/test-pdf-coverage.sh. +# +# Usage: +# ./tools/test-email-coverage.sh +# EMAIL_COVERAGE_REPORT_ONLY=1 ./tools/test-email-coverage.sh +# +# Runs entirely inside one temporary directory (its own rustup, cargo home +# and target dir), removed on exit. It does not touch ~/.cargo, the workspace +# target directory or any source file. + +set -Eeuo pipefail +IFS=$'\n\t' + +ROOT="$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +TOOLCHAIN="${RUST_TOOLCHAIN:-1.97.1}" + +# The whole-domain floor, set a little under today's measurement (~93%) so +# ordinary refactoring does not trip it while a real loss of coverage does. +TOTAL_FLOOR="${EMAIL_COVERAGE_TOTAL_FLOOR:-90}" + +# Per-file floors for the files that have actually harboured the bugs the +# remediation fixed. A single whole-domain number hides exactly the failure +# this is meant to catch: a regression in the proxy parser or the recipient +# splitter moves the total by less than a point. Each entry is "path:floor". +# +# email_worker's floor is lower because its uncovered lines are the actual +# SMTP I/O and the wasm `#[cfg]` blocks -- genuinely untestable without a +# live server or a browser (plan "What I have not verified"). The same +# reasoning puts mail_proxy's floor a little under its measured value: its +# remaining gaps are the reqwest/fetch transports, not the parsing logic. +PER_FILE_FLOORS="${EMAIL_COVERAGE_PER_FILE_FLOORS:-\ +nigig-core/src/mail_proxy.rs:85 +nigig-core/src/mail_backend.rs:92 +nigig-core/src/email_worker.rs:65 +nigig-core/src/email_send.rs:95 +nigig-core/src/email_store.rs:95 +nigig-core/src/email_account.rs:95 +nigig-core/src/secret.rs:95}" + +WORK="$(mktemp -d "${TMPDIR:-/tmp}/email-coverage.XXXXXXXX")" +cleanup() { + local status=$? + if [ "${KEEP_TEST_ENV:-0}" = "1" ]; then + printf 'kept coverage environment at %s\n' "$WORK" >&2 + else + chmod -R u+w "$WORK" 2>/dev/null || true + rm -rf "$WORK" + fi + exit "$status" +} +trap cleanup EXIT HUP INT TERM + +export RUSTUP_HOME="$WORK/rustup" +export CARGO_HOME="$WORK/cargo" +export CARGO_TARGET_DIR="$WORK/target" +export PATH="$CARGO_HOME/bin:$PATH" +export LLVM_PROFILE_FILE="$WORK/profiles/%p-%m.profraw" +# `-C codegen-units=1` is deliberately NOT set: the email domain is compiled +# alongside Makepad (naga, etc.) and a single codegen unit on those crates +# spikes memory hard enough to get OOM-killed on small runners. Line +# coverage is unaffected; only edge/region precision would be. +export RUSTFLAGS="-C instrument-coverage -C opt-level=0" +# nigig-core depends on the Makepad fork (a large git repo). libgit2 can fail +# to fetch it into a fresh CARGO_HOME with "unrecoverable internal error: +# 'writer.open == 0'"; using the system git CLI for fetches avoids that. +export CARGO_NET_GIT_FETCH_WITH_CLI=true +mkdir -p "$WORK/profiles" + +printf 'installing Rust %s with llvm-tools\n' "$TOOLCHAIN" +curl --fail --silent --show-error --location https://sh.rustup.rs \ + -o "$WORK/rustup-init" +chmod 700 "$WORK/rustup-init" +"$WORK/rustup-init" -y --profile minimal --default-toolchain "$TOOLCHAIN" \ + --component llvm-tools-preview --no-modify-path >/dev/null + +printf 'running the email domain tests under instrumentation\n' +cd "$ROOT" +cargo test --locked -p nigig-core --lib -- \ + email_ secret:: mail_backend:: mail_proxy:: >"$WORK/test.log" 2>&1 \ + || { cat "$WORK/test.log"; exit 1; } +grep -E 'test result' "$WORK/test.log" | tail -5 + +HOST="$(rustc -vV | sed -n 's/^host: //p')" +LLVM_BIN="$RUSTUP_HOME/toolchains/$TOOLCHAIN-$HOST/lib/rustlib/$HOST/bin" + +"$LLVM_BIN/llvm-profdata" merge -sparse "$WORK/profiles"/*.profraw \ + -o "$WORK/coverage.profdata" + +# The lib test binary is the one that ran the email domain tests. nigig-core +# is a single crate, so unlike the three-crate PDF engine a single binary is +# complete, not an under-report. +BIN="$(find "$CARGO_TARGET_DIR/debug/deps" -maxdepth 1 -type f -executable \ + -name 'nigig_core-*' | head -1)" +if [ -z "$BIN" ]; then + echo 'no nigig-core test binary was produced' >&2 + exit 1 +fi + +# Only the seven email files count. Makepad's generated code, other +# nigig-core modules and the registry are all excluded. +IGNORE='(/cargo/registry|/cargo/git|/rustc/)' + +printf '\n=== per-file coverage ===\n' +"$LLVM_BIN/llvm-cov" report "$BIN" \ + -instr-profile="$WORK/coverage.profdata" \ + -ignore-filename-regex="$IGNORE" \ + "$ROOT/crates/nigig-core/src/email_account.rs" \ + "$ROOT/crates/nigig-core/src/email_send.rs" \ + "$ROOT/crates/nigig-core/src/email_store.rs" \ + "$ROOT/crates/nigig-core/src/email_worker.rs" \ + "$ROOT/crates/nigig-core/src/mail_backend.rs" \ + "$ROOT/crates/nigig-core/src/mail_proxy.rs" \ + "$ROOT/crates/nigig-core/src/secret.rs" | tee "$WORK/report.txt" + +# Machine-readable totals, filtered to the seven email files. The export +# covers every file in the binary, so the floor is computed over exactly +# the files this script reported on, matched by path suffix. +"$LLVM_BIN/llvm-cov" export "$BIN" \ + -instr-profile="$WORK/coverage.profdata" \ + -ignore-filename-regex="$IGNORE" > "$WORK/coverage.json" + +if [ "${EMAIL_COVERAGE_REPORT_ONLY:-0}" = "1" ]; then + echo 'report-only mode: the floor was not enforced' + exit 0 +fi + +python3 - "$WORK/coverage.json" "$TOTAL_FLOOR" "$PER_FILE_FLOORS" <<'PY' +import json, sys + +path, total_floor, per_file = sys.argv[1], float(sys.argv[2]), sys.argv[3] +with open(path) as fh: + data = json.load(fh) + +keep = ("email_account.rs", "email_send.rs", "email_store.rs", + "email_worker.rs", "mail_backend.rs", "mail_proxy.rs", "secret.rs") +files = [f for f in data["data"][0]["files"] + if f["filename"].endswith(keep)] + +total_lines = sum(f["summary"]["lines"]["count"] for f in files) +covered = sum(f["summary"]["lines"]["covered"] for f in files) +total = 100.0 * covered / total_lines if total_lines else 0.0 +print(f"\ntotal line coverage over the email domain: {total:.2f}% " + f"(floor {total_floor:.2f}%)") + +failures = [] +if total < total_floor: + failures.append(f" total {total:.2f}% is below the floor of " + f"{total_floor:.2f}%") + +measured = {} +for f in files: + measured[f["filename"]] = f["summary"]["lines"]["percent"] + +for line in per_file.split(): + if not line.strip(): + continue + name, _, floor = line.rpartition(":") + floor = float(floor) + matches = [v for k, v in measured.items() if k.endswith(name)] + if not matches: + failures.append( + f" {name} has a floor but was not measured - was it renamed or " + "deleted? A floor on a file that no longer exists silently " + "protects nothing.") + continue + got = matches[0] + if got < floor: + failures.append(f" {name} {got:.2f}% is below its floor of " + f"{floor:.2f}%") + +if failures: + print("coverage floors not met:", file=sys.stderr) + print("\n".join(failures), file=sys.stderr) + print( + "\nEither the change removed tested code, or it added untested code.\n" + "Lowering a floor is a reviewable edit to tools/test-email-coverage.sh,\n" + "not something to do quietly.", + file=sys.stderr, + ) + sys.exit(1) + +print(f"all coverage floors met (total {total:.2f}%)") +PY