diff --git a/.forgejo/workflows/email.yml b/.forgejo/workflows/email.yml index 14373ae..1bb7c72 100644 --- a/.forgejo/workflows/email.yml +++ b/.forgejo/workflows/email.yml @@ -24,13 +24,8 @@ 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' @@ -39,13 +34,8 @@ 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' @@ -379,9 +369,7 @@ jobs: # --------------------------------------------------------------------- email-domain: runs-on: ubuntu-latest - # Two full compiles now (plain tests + the instrumented coverage run), - # so allow more than the pre-coverage 30 minutes. - timeout-minutes: 45 + timeout-minutes: 30 steps: - uses: actions/checkout@v4 @@ -407,24 +395,15 @@ jobs: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - name: Email domain tests - run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy::" + run: "cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend::" # A floor, not a ratchet: these tests are cheap, pure, and the - # number should only go up. 38 at Phase 0; 154 after C1c/C1d. + # number should only go up. 38 today. - name: The email domain test suite must not shrink run: | set -euo pipefail - 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 + FLOOR=120 + out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: 2>&1)" 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 e99d442..78ff21b 100644 --- a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -529,18 +529,16 @@ 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, 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 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 B is complete.** B1 was the live Critical: the field labelled "To (comma-separated)" was parsed by @@ -763,8 +761,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~~ | **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. | +| 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. | | 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 2aa18fd..922cc6d 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,18 +6,11 @@ // 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. // -// 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. +// All validation goes through nigig_core::email_account::AccountDraft, +// which is unit tested on the host. This widget only moves strings. use makepad_widgets::*; -use nigig_core::mail_backend::{BackendDraft, BackendKind, SetupDraft}; +use nigig_core::email_account::{guess_provider, AccountDraft}; script_mod! { use mod.prelude.widgets.* @@ -47,53 +40,12 @@ script_mod! { } Label { width: Fill, height: Fit - text: "Add an account to read your inbox and send mail." + text: "Add an account to read your inbox and send mail. Your password stays on this device for the current session only." draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 11.0 } } } } - 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 { + form_card := RoundedView { width: Fill, height: Fit flow: Down spacing: 8 @@ -108,48 +60,15 @@ script_mod! { 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 { - 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 { + Label { text: "SMTP server" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + 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: "SMTP port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } - smtp_port_input := TextInput { + Label { text: "Port" draw_text +: { color: #x64748B, text_style: theme.font_regular { font_size: 10.5 } } } + port_input := TextInput { width: Fill, height: 42 text: "587" empty_text: "587" @@ -170,67 +89,38 @@ script_mod! { is_password: true draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - } - 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 { + 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: "https://mail.example.com/api" + empty_text: "Shown to people you email" draw_bg +: { border_radius: 10.0, border_size: 1.0, border_color: #xE2E8F0 } } - 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 } + 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: "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 } + setup_status := Label { + width: Fill, height: Fit + text: "" + draw_text +: { color: #xB4232C, text_style: theme.font_regular { font_size: 10.5 } } } } - - 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 setup draft -/// (identity + backend choice); the screen validates and drives the -/// connection test for whichever backend was picked. +/// Emitted when the user asks to connect. Carries the raw draft; the +/// screen validates and drives the connection test. #[derive(Clone, Debug, Default)] pub enum EmailAccountSetupAction { #[default] None, - /// 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), + Connect(AccountDraft), } impl ActionDefaultRef for EmailAccountSetupAction { @@ -244,12 +134,8 @@ impl ActionDefaultRef for EmailAccountSetupAction { pub struct EmailAccountSetup { #[deref] view: View, - /// 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. + /// Set when the address field last auto-filled the server, so a user + /// who typed their own server is never overwritten. #[rust] autofilled_server: bool, } @@ -262,26 +148,30 @@ impl Widget for EmailAccountSetup { return; }; - // 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. + // 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. if self .view .text_input(cx, ids!(address_input)) .changed(actions) .is_some() { - self.autofill_provider(cx); + 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); + } + } } if self.view.button(cx, ids!(connect_btn)).clicked(actions) { @@ -303,10 +193,7 @@ 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(Box::new(draft)), - ); + cx.widget_action(self.widget_uid(), EmailAccountSetupAction::Connect(draft)); } } } @@ -318,93 +205,14 @@ impl Widget for EmailAccountSetup { } impl EmailAccountSetup { - /// 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(), - ..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 { + 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(), + username: self.view.text_input(cx, ids!(username_input)).text(), + password: self.view.text_input(cx, ids!(password_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(), - }, } } @@ -417,47 +225,23 @@ impl EmailAccountSetup { } /// Prefill from a previously-entered account after a failure, so the - /// user fixes one field instead of retyping the lot. The secret is - /// deliberately not restored -- it is session-only. + /// user fixes one field instead of retyping six. 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 d225c9e..322aae0 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs @@ -20,15 +20,12 @@ // with no path to any mail, and the credentials form was on the Bulk tab. use makepad_widgets::*; -use nigig_core::email_account::{EmailAccount, SessionState}; +use nigig_core::email_account::{AccountDraft, 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_proxy_verify, spawn_smtp_test, EmailWorkerAction, SmtpConfig, -}; -use nigig_core::mail_backend::{BackendSettings, SetupDraft}; +use nigig_core::email_worker::{spawn_smtp_test, EmailWorkerAction, SmtpConfig}; use nigig_core::secret::Secret; use nigig_uikit::shared::conversation::conversation_preview::{ SharedConversationPreviewAction, SharedConversationPreviewProps, @@ -214,21 +211,16 @@ 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); } } - // Connection test result (SMTP or proxy) decides signed-in vs - // failed. Both carry a plain `Result<(), String>`, so the flow is - // transport-agnostic. + // SMTP test result decides signed-in vs failed. 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()); - } } } @@ -310,10 +302,9 @@ fn format_thread_time(ms: i64) -> String { } impl EmailInboxPage { - /// 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() { + /// 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() { Ok(v) => v, Err(errors) => { let msg = errors @@ -326,49 +317,36 @@ impl EmailInboxPage { } }; - let account = validated.account.clone(); self.session = SessionState::Verifying; self.pending = Some(account.clone()); - self.password = validated.secret.clone(); + self.password = password.clone(); - 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); - } + // 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); } + + // 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 ea425a0..e922885 100644 --- a/crates/nigig-core/src/email_worker.rs +++ b/crates/nigig-core/src/email_worker.rs @@ -102,34 +102,10 @@ 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. @@ -537,10 +513,6 @@ 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, } @@ -618,39 +590,6 @@ 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 234c2ef..9a09cc4 100644 --- a/crates/nigig-core/src/lib.rs +++ b/crates/nigig-core/src/lib.rs @@ -23,7 +23,6 @@ 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 4d836b3..3e383f2 100644 --- a/crates/nigig-core/src/mail_backend.rs +++ b/crates/nigig-core/src/mail_backend.rs @@ -349,124 +349,6 @@ 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 @@ -564,7 +446,6 @@ pub fn backend_from(settings: BackendSettings, secret: Secret) -> Box BackendDraft { BackendDraft { @@ -824,20 +705,6 @@ 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"); @@ -959,133 +826,4 @@ 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 deleted file mode 100644 index aafc421..0000000 --- a/crates/nigig-core/src/mail_proxy.rs +++ /dev/null @@ -1,745 +0,0 @@ -//! 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 deleted file mode 100644 index 0d6933f..0000000 --- a/tools/test-email-coverage.sh +++ /dev/null @@ -1,196 +0,0 @@ -#!/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