diff --git a/.forgejo/workflows/email.yml b/.forgejo/workflows/email.yml index 6f6ca3f..08f43d2 100644 --- a/.forgejo/workflows/email.yml +++ b/.forgejo/workflows/email.yml @@ -78,86 +78,6 @@ jobs: fi echo "OK" - # A1 / S2. SmtpConfig used to carry `pub password: String` and derive - # Serialize, so on wasm the entire struct -- password included -- was - # serde_json-encoded and POSTed to the email API. Any proxy logging - # request bodies captured the credential. - # - # Two properties must hold, and both are structural rather than - # advisory: - # 1. the password field is a `Secret`, whose Debug renders "***"; - # 2. SmtpConfig does NOT derive Serialize, so it cannot be - # serialised wholesale by accident. The wasm path builds its - # JSON field by field instead, naming the secret at exactly one - # line. - - name: The SMTP password must be a Secret, and the config unserialisable - run: | - set -euo pipefail - bad=0 - decl=$(sed -n '/^pub struct SmtpConfig {/,/^}/p' \ - crates/nigig-core/src/email_worker.rs \ - | grep -E '^[[:space:]]*pub[[:space:]]+password' || true) - if ! echo "$decl" | grep -q 'Secret'; then - echo "ERROR: SmtpConfig.password is not a Secret:" - echo " $decl" - echo "A plain String Debug-prints and serialises the credential." - bad=1 - fi - # The derive list immediately above the struct. - derives=$(grep -B2 '^pub struct SmtpConfig {' \ - crates/nigig-core/src/email_worker.rs | grep '#\[derive' || true) - if echo "$derives" | grep -qE 'Serialize|Deserialize'; then - echo "ERROR: SmtpConfig derives Serialize/Deserialize:" - echo " $derives" - echo "That is how the plaintext password reached the wire. Build" - echo "the request body field by field instead." - bad=1 - fi - [ "$bad" -eq 0 ] || exit 1 - echo "OK" - - # A5. The wasm proxy POST carries the credential, so the endpoint must - # be same-origin or https. The setter used to accept any String, - # including http://, which sends the password in clear text. - - name: The email API endpoint must be validated before use - run: | - set -euo pipefail - if ! grep -q 'fn email_api_url_is_safe' \ - crates/nigig-core/src/email_worker.rs; then - echo "ERROR: email_api_url_is_safe() is gone." - echo "set_email_api_url must reject non-https absolute URLs." - exit 1 - fi - if ! grep -q 'email_api_url_is_safe(&url)' \ - crates/nigig-core/src/email_worker.rs; then - echo "ERROR: set_email_api_url no longer calls the validator." - exit 1 - fi - echo "OK" - - # A3. The TLS mode was implicit -- inherited from lettre's defaults via - # a bare port match. Those defaults are safe, but nothing asserted it, - # so a refactor could have removed encryption with no test failing. - # Every port must map to a TLS mode; there is no cleartext arm. - - name: SMTP transport must never be cleartext - run: | - set -euo pipefail - if ! grep -q 'fn tls_mode_for_port' \ - crates/nigig-core/src/email_worker.rs; then - echo "ERROR: tls_mode_for_port() is gone; the TLS policy is" - echo "implicit again." - exit 1 - fi - if grep -nE 'Tls::None|Tls::Opportunistic' \ - crates/nigig-core/src/email_worker.rs; then - echo - echo "ERROR: a non-mandatory TLS mode above. This transport always" - echo "carries credentials; Opportunistic silently accepts a" - echo "downgrade." - exit 1 - fi - echo "OK" - # B3. `port.parse().unwrap_or(587)` silently rewrote a typo'd port, # and because the port selects the transport (465 implicit TLS vs # 587 STARTTLS) that silently changed the security posture too. @@ -269,15 +189,15 @@ jobs: echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - name: Email domain tests - run: "cargo test --locked -p nigig-core --lib -- email_ secret::" + run: cargo test --locked -p nigig-core --lib email_ # A floor, not a ratchet: these tests are cheap, pure, and the # number should only go up. 38 today. - name: The email domain test suite must not shrink run: | set -euo pipefail - FLOOR=60 - out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: 2>&1)" + FLOOR=38 + out="$(cargo test --locked -p nigig-core --lib email_ 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 d3853e2..a3167be 100644 --- a/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md +++ b/REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md @@ -528,17 +528,7 @@ snapshot. Commits are on `main`. | `4ae50cb` | **Phase 0.5 DONE** — `serde`, `serde_json`, `robius-location` removed; platform-dep gate extended to `nigig-email`. | | `244c4f2` | **Phase 0.3 + 0.6 DONE** — `email.yml` (4 jobs, 11 gates). Writing the gates caught **B2 and B3 still live in `bulk.rs`**; both fixed here. | -| *(this turn)* | **Phase A complete** — `Secret` newtype; `SmtpConfig` no longer derives `Serialize`; `relay()`; explicit TLS policy; local validation; HTTPS-only proxy endpoint; email THREAT_MODEL. Domain tests **38 → 68**. | - **Phase 0 is complete.** All seven items done; 0.7 was fixed upstream. - -**Phase A is complete.** All six items done. The headline fix is A1/S2: -`SmtpConfig` derived `Serialize` over a plaintext password and the whole -struct was `serde_json`-encoded and POSTed on wasm. It now holds a -`Secret` (Debug renders `"***"`, no `Display`, no `Serialize`), the -derive is gone, and the wasm body is built field by field so the -credential appears at exactly one line. Three CI gates enforce it, each -negative-tested. Every gate in `email.yml` was negative-tested — reverted the fix, confirmed the gate fails, restored it — rather than merely observed green. @@ -642,12 +632,12 @@ Exit: `cargo check`/`clippy`/`test -p nigig-email` green on a real runner. | ID | Task | |---|---| -| ~~A1~~ | **DONE** — `Secret` newtype in `nigig-core/src/secret.rs`. Redacting `Debug` (`"***"`), `#[serde(skip)]`. **CI gate**: the password field must never be plainly serialisable — mirrors the SMS `#[serde(skip)]` body gate. *This is the real critical item; do it first.* | -| ~~A2~~ | **DONE** — now uses `relay()`. Behaviour-preserving today (verified equivalent), but stops the code reading as a vulnerability and inherits future hardening. Add a test asserting transport choice per port. | -| ~~A3~~ | **DONE** — `tls_mode_for_port` + tests., so the inherited defaults cannot be silently removed: assert `Tls::Wrapper`/`Tls::Required` per port and never `Tls::None`. | -| ~~A4~~ | **DONE** — `validate_send` + `config_warning`.: non-empty server/username/from, `from` parses, bounded subject (≤998 bytes per RFC 5322) and body, recipient count cap. All host-testable. | -| ~~A5~~ | **DONE** — `email_api_url_is_safe`.; reject non-HTTPS absolute URLs. Document that the wasm endpoint needs auth. | -| ~~A6~~ | **DONE** — `crates/apps/nigig-email/THREAT_MODEL.md`. for email: what the client can enforce, and what it cannot (`from` spoofing is server-side, per SMS E11). | +| A1 | **`Secret` newtype for the password.** Redacting `Debug` (`"***"`), `#[serde(skip)]`. **CI gate**: the password field must never be plainly serialisable — mirrors the SMS `#[serde(skip)]` body gate. *This is the real critical item; do it first.* | +| A2 | **Replace the hand-rolled 465 branch with `relay()`.** Behaviour-preserving today (verified equivalent), but stops the code reading as a vulnerability and inherits future hardening. Add a test asserting transport choice per port. | +| A3 | **Pin the TLS policy with a test**, so the inherited defaults cannot be silently removed: assert `Tls::Wrapper`/`Tls::Required` per port and never `Tls::None`. | +| A4 | **Validate before dispatch**: non-empty server/username/from, `from` parses, bounded subject (≤998 bytes per RFC 5322) and body, recipient count cap. All host-testable. | +| A5 | **Scheme-check `set_email_api_url`**; reject non-HTTPS absolute URLs. Document that the wasm endpoint needs auth. | +| A6 | **Add a `THREAT_MODEL.md` section** for email: what the client can enforce, and what it cannot (`from` spoofing is server-side, per SMS E11). | Negative tests: remove `#[serde(skip)]` → A1 gate fails. Set `Tls::None` on either branch → A3 test fails. diff --git a/crates/apps/nigig-email/THREAT_MODEL.md b/crates/apps/nigig-email/THREAT_MODEL.md deleted file mode 100644 index f857d4f..0000000 --- a/crates/apps/nigig-email/THREAT_MODEL.md +++ /dev/null @@ -1,216 +0,0 @@ -# nigig-email — Threat Model - -Scope: `crates/apps/nigig-email` and the email modules it depends on in -`crates/nigig-core` (`email_account`, `email_store`, `email_worker`, -`secret`). - -Separate from the root `THREAT_MODEL.md`, which is Nigig-Pay's and shares -no assets with this feature. - -Written as part of Phase A of -`REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md`. Each mitigation names the -control, so a reader can check the claim rather than take it on trust. - ---- - -## 1. Assets - -| # | Asset | Why it matters | -|---|---|---| -| A1 | **The user's mailbox password or app password** | The highest-value secret in the app. For most people email is the password-reset channel for every other account they own, so losing it is worse than losing a payment credential. | -| A2 | Message bodies and subjects | Private correspondence. May contain financial and identity data. | -| A3 | The sender's address and contact graph | Who the user talks to is sensitive even when content is not. | -| A4 | SMTP/IMAP server names and ports | Low value alone; useful for targeting. | -| A5 | The proxy API endpoint and its token (once C1d lands) | A revocable stand-in for A1 — strictly safer, because revoking it does not require a password change. | - -## 2. Trust boundaries - -``` - ┌─ device ───────────────────────────────────┐ - │ UI widgets ──▶ nigig-core (validation) │ - │ │ │ - │ ▼ │ - │ Secret (in memory only) │ - └──────────────────────┼─────────────────────┘ - │ ← boundary 1: the network - ┌───────────────┴────────────────┐ - ▼ ▼ - SMTP relay (native) Proxy API (wasm) - TLS required https required -``` - -Boundary 1 is the one that matters. Everything above it is our code; -everything below is a third party we authenticate to with A1. - -## 3. Threats and mitigations - -### T-E1 — Password captured in transit (CRITICAL) - -*An attacker on the network reads the credential during AUTH.* - -Mitigated: - -- Every port maps to a mandatory TLS mode; `tls_mode_for_port` has **no - cleartext arm**. Port 465 → implicit TLS via `relay()`; everything else - → `starttls_relay`, which uses `Tls::Required` and so aborts **before** - sending AUTH if the upgrade fails. That is what defeats a downgrade. -- Certificate and hostname validation are on (`TlsParameters::new` sets - `accept_invalid_certs: false`, `accept_invalid_hostnames: false`, TLS - 1.2 floor). The `dangerous_*` opt-outs are never called. -- A CI gate fails the build if `Tls::None` or `Tls::Opportunistic` appears, - or if `tls_mode_for_port` is deleted. -- On wasm, `set_email_api_url` rejects anything that is not same-origin or - `https://`, **including protocol-relative `//host/path`**, which is http - on an http page and is easy to mistake for a relative path. - -Residual: we trust the platform root store. A device with an attacker- -installed CA can still intercept. Certificate pinning is not implemented. - -### T-E2 — Password leaked by our own code (CRITICAL) - -*The credential escapes through a log, a crash dump, or a serialised -struct — not through an attack, but through carelessness.* - -This was **live**, not hypothetical. `SmtpConfig` derived `Serialize` over -a plaintext `String`, and on wasm the whole struct was `serde_json`-encoded -and POSTed. Any reverse proxy or APM tool logging request bodies captured -it, and nothing in the code said so. - -Mitigated: - -- `Secret` (`crates/nigig-core/src/secret.rs`): `Debug` always renders - `Secret("***")`, with no verbose mode. `Display` is **not implemented**, - so `format!("{s}")` will not compile either. -- `Serialize`/`Deserialize` are **not implemented** on `Secret` and have - been **removed from `SmtpConfig`**. A struct holding a secret cannot be - serialised wholesale; the compiler stops it. The wasm request body is - now assembled field by field, so `password` appears at exactly **one - line** and "what leaves the device?" is answerable by reading one - function. -- `EmailAccount` — the persistable half — has **no password field at all**. - Two controls: a unit test asserting the serialised form contains neither - the secret nor a field named `password`, and a CI gate on the struct - body in case someone deletes the test. -- `expose()` is the single reader, named to be conspicuous in review. -- The secret is cleared the moment a connection is known to have failed. - -**Residual, and worth stating plainly:** `Secret` does **not** zero its -buffer on drop. Without a `zeroize`-style crate the plaintext can persist -in freed heap memory, and on a device with swap it can reach disk. The -type is a leak-through-code control, not an anti-forensics one. - -### T-E3 — Password persisted in cleartext (HIGH) - -*A "remember my settings" feature writes A1 to a JSON file.* - -Not currently possible: nothing persists the credential, and `Secret` -cannot be serialised. That is presently **by omission** rather than by -design — the safest property of the original code was that it never -implemented storage. - -**Open.** Plan item **C1f** must use the platform keystore. The in-repo -precedent is `robius-sms`'s `SmsScheduleCrypto.java` — AES-256-GCM via -`AndroidKeyStore`, failing closed. Until then, a restart requires -re-entering the password, which is the correct trade. - -### T-E4 — Header or content injection (MEDIUM) - -*`\r\n` in a subject or recipient forges headers (BCC, Reply-To).* - -Partly mitigated: - -- `lettre`'s `Message::builder()` encodes headers, so classic CRLF - injection is handled **by the library**, not by us. -- `validate_send` bounds subject at 998 bytes (RFC 5322) and body at 5 MB, - both before any network call. - -Residual: we do not reject control characters in the subject ourselves. We -rely on `lettre`. If the transport is ever swapped — e.g. for the C1d -proxy, which does **not** go through `lettre` — this becomes unmitigated -and the proxy body builder must sanitise. - -### T-E5 — Sender spoofing (MEDIUM, not fixable here) - -*A user sets `from` to an address they do not control.* - -**Cannot be mitigated client-side.** Whether `from` may differ from the -authenticated identity is the relay's policy (SPF/DKIM/DMARC), enforced -server-side. Same conclusion as SMS **E11**. - -What we can do, and do not yet: warn when `from` differs from `username`, -which is the most common cause of a silent provider rejection. **Open**, -folded into A4 follow-up. - -### T-E6 — Unauthenticated proxy endpoint (MEDIUM) - -*Any script on the page drives the send endpoint using the user's -session.* - -Partly mitigated: the endpoint must be https or same-origin (T-E1). - -**Open.** The POST still carries no auth token, no CSRF token and no -request signing. This is a **C1d requirement**, not an afterthought: the -proxy is only safer than on-device IMAP if the token is revocable and -scoped. - -### T-E7 — Denial of service via a hostile message (MEDIUM) - -*A crafted message body crashes the client on render.* - -Mitigated: - -- `preview_line` slices on `char_indices`, never byte offsets. This is the - SMS **A3** bug class: `&s[..n]` panics when `n` is not a UTF-8 boundary, - and one inbound message containing emoji or non-Latin text took down the - whole SMS list on **every frame** until deleted. Email bodies are more - hostile, not less — arbitrary MIME from anyone who knows the address. -- Tested against emoji, Swahili, Arabic, Japanese and deliberately - misaligned mixed text, which is the case that actually triggers it - (uniform emoji happens to land on a boundary). -- A CI gate rejects new byte-offset slicing in the email text helpers. -- Timestamp formatting is total over `i64`, including `MIN`/`MAX`, because - it runs inside `draw_walk` per visible row. - -### T-E8 — Unbounded network operation (LOW) - -*A black-holed relay hangs the UI.* - -Mitigated: a 20-second SMTP timeout, shorter than `lettre`'s 60s-per- -command default. `validate_send` also refuses obviously incomplete -configs locally, so the common failure costs no round trip. - -Residual: no cancel button, and tasks are fire-and-forget with no retained -`JoinHandle`. Plan item **B6**. - -### T-E9 — Accidental duplicate send (LOW) - -*A double tap sends twice — billed, irreversible, to a human.* - -**Open.** No in-flight guard and no confirmation. `robius-sms` solved this -with `BULK_SEND_IN_FLIGHT` (an `AtomicBool` swap) plus two-tap -confirmation; email has the same irreversibility and neither control. -Plan item **B5**. - -## 4. Open risks, ranked - -| Risk | Plan item | Why it is still open | -|---|---|---| -| No keystore-backed credential storage | C1f | Needs platform work; deliberately blocks "remember me" until then | -| Proxy endpoint unauthenticated | C1d | Must land with the proxy backend, not after | -| `Secret` does not zero on drop | — | Needs a `zeroize` dependency; not yet justified, but do not claim the protection | -| No duplicate-send guard | B5 | Small; next phase | -| No cancel on a running send | B6 | Small; next phase | -| `from` ≠ `username` not warned | A4 follow-up | Cheap, high-value diagnostic | -| No certificate pinning | — | Accepted: platform root store is the norm for mail clients | - -## 5. What has not been tested - -Stated because a threat model that overclaims is worse than none: - -- **No live SMTP server has been contacted.** The TLS analysis is a read of - `lettre` 0.11.23's source, plus unit tests on our own port→mode mapping. - No interception has been attempted. -- **The wasm path has not been built or run.** `call_email_api` is - `#[cfg(target_arch = "wasm32")]`; `email_api_url_is_safe` is host-tested, - but the fetch code around it is not. -- **No IMAP code exists yet**, so T-E1/T-E2 cover the SMTP direction only. 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..6b4c1b9 100644 --- a/crates/apps/nigig-email/src/email_frame/pages/inbox.rs +++ b/crates/apps/nigig-email/src/email_frame/pages/inbox.rs @@ -26,7 +26,6 @@ use nigig_core::email_store::{ EmailThreadSummary, }; use nigig_core::email_worker::{spawn_smtp_test, EmailWorkerAction, SmtpConfig}; -use nigig_core::secret::Secret; use nigig_uikit::shared::conversation::conversation_preview::{ SharedConversationPreviewAction, SharedConversationPreviewProps, }; @@ -155,10 +154,8 @@ pub struct EmailInboxPage { /// Session-only. Never written to disk -- see the note in /// `nigig_core::email_account`: `EmailAccount` is the persistable half /// and deliberately has no password field. - /// - /// A1: a `Secret`, so a Debug-print of this widget cannot leak it. #[rust] - password: Secret, + password: String, /// The draft awaiting a connection result, so a failure can report /// against the account the user actually typed. #[rust] @@ -321,21 +318,6 @@ impl EmailInboxPage { self.pending = Some(account.clone()); self.password = password.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); - } - // 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 diff --git a/crates/nigig-core/src/email_account.rs b/crates/nigig-core/src/email_account.rs index 7d7818a..fbcd995 100644 --- a/crates/nigig-core/src/email_account.rs +++ b/crates/nigig-core/src/email_account.rs @@ -21,7 +21,6 @@ //! AES-256-GCM/AndroidKeyStore path is the in-repo precedent) without //! changing anything here. -use crate::secret::Secret; use serde::{Deserialize, Serialize}; /// A configured email account, minus the secret. @@ -134,7 +133,7 @@ impl AccountDraft { /// Returns every error found, not just the first, so the form can /// mark all bad fields in one pass instead of making the user /// resubmit five times. - pub fn validate(&self) -> Result<(EmailAccount, Secret), Vec> { + pub fn validate(&self) -> Result<(EmailAccount, String), Vec> { let mut errors = Vec::new(); let address = self.address.trim(); @@ -199,7 +198,7 @@ impl AccountDraft { username, display_name: self.display_name.trim().to_string(), }, - Secret::new(self.password.clone()), + self.password.clone(), )) } } @@ -302,7 +301,7 @@ mod tests { let (account, secret) = good_draft().validate().expect("should validate"); assert_eq!(account.address, "jane@example.com"); assert_eq!(account.smtp_port, 587); - assert_eq!(secret.expose(), "hunter2"); + assert_eq!(secret, "hunter2"); } /// The persistable half must never carry the secret. This is the @@ -381,7 +380,7 @@ mod tests { ..good_draft() }; let (_, secret) = draft.validate().unwrap(); - assert_eq!(secret.expose(), " spaced "); + assert_eq!(secret, " spaced "); } #[test] diff --git a/crates/nigig-core/src/email_worker.rs b/crates/nigig-core/src/email_worker.rs index d192d8e..421ecb5 100644 --- a/crates/nigig-core/src/email_worker.rs +++ b/crates/nigig-core/src/email_worker.rs @@ -1,189 +1,44 @@ + use makepad_widgets::*; +use serde::{Deserialize, Serialize}; -use crate::secret::Secret; - -/// SMTP connection settings for one send. -/// -/// A1 / assessment finding S2. This type used to derive `Serialize`, -/// `Deserialize` and `Debug` over a plaintext `password: String`. On wasm -/// the whole struct was `serde_json`-encoded and POSTed to the email API, -/// so every request carried the password in clear text -- and any proxy -/// logging request bodies captured it. -/// -/// Two changes, both structural rather than advisory: -/// -/// * the password is a `Secret`, whose `Debug` always renders `"***"`, -/// so printing the config cannot leak it; -/// * `Serialize`/`Deserialize` are GONE. The wasm path now builds its -/// JSON field by field and names the secret at exactly one line, so -/// "does this leave the device?" is answerable by reading one -/// function instead of trusting a derive. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct SmtpConfig { pub server: String, pub port: u16, pub username: String, - pub password: Secret, + pub password: String, pub from: String, } -impl SmtpConfig { - /// Standard submission port (STARTTLS). `Default` gives port 0, which - /// is never valid, so callers that do not set a port explicitly should - /// use this. - pub const DEFAULT_PORT: u16 = 587; - - /// True when this config cannot possibly authenticate. Cheap guard - /// before spending a network round trip (A4). - pub fn is_incomplete(&self) -> bool { - self.server.trim().is_empty() - || self.username.trim().is_empty() - || self.from.trim().is_empty() - || self.password.is_blank() - || self.port == 0 +impl Default for SmtpConfig { + fn default() -> Self { + Self { + server: String::new(), + port: 587, + username: String::new(), + password: String::new(), + from: String::new(), + } } } /// Configure the HTTP API endpoint used for email on wasm. No-op on native. -/// -/// A5: rejects anything that is not either a same-origin relative path or -/// an `https://` URL. The old setter took any `String`, so a caller could -/// point the credential-bearing POST at `http://` and have it sent in -/// clear text over the network -- which is the one thing the proxy design -/// is supposed to prevent. -/// -/// Returns whether the URL was accepted, so a caller that ignores the -/// result still cannot silently downgrade: the endpoint simply stays at -/// its safe default. #[cfg(target_arch = "wasm32")] -pub fn set_email_api_url(url: String) -> bool { - if !email_api_url_is_safe(&url) { - return false; - } - EMAIL_API_URL.set(url).is_ok() -} - -/// Is this an endpoint we are willing to send credentials to? -/// -/// Split out from the setter so it is testable on the host -- the wasm -/// target cannot run `cargo test` here, and an unvalidated validator is -/// not a control. -pub fn email_api_url_is_safe(url: &str) -> bool { - let u = url.trim(); - if u.is_empty() { - return false; - } - // Relative, same-origin: inherits the page's scheme, which is https - // wherever this is deployed over https. - if u.starts_with('/') && !u.starts_with("//") { - return true; - } - // Absolute: https only. Note `//host/path` is protocol-relative and - // therefore http on an http page, so it is rejected above. - u.starts_with("https://") && u.len() > "https://".len() +pub fn set_email_api_url(url: String) { + EMAIL_API_URL.set(url).ok(); } /// Spawn an async SMTP test task. Posts `EmailWorkerAction::SmtpTestResult` on completion. -/// -/// A4: refuses an incomplete config locally instead of spending a network -/// round trip to be told the obvious. The old code spawned regardless, so -/// an empty server produced a raw `lettre` DNS error the user could not -/// act on. pub fn spawn_smtp_test(config: SmtpConfig) { - if config.is_incomplete() { - Cx::post_action(EmailWorkerAction::SmtpTestResult(Err( - INCOMPLETE_CONFIG_MESSAGE.to_string(), - ))); - return; - } crate::platform::spawn(async move { let result = smtp_test_impl(&config).await; Cx::post_action(EmailWorkerAction::SmtpTestResult(result)); }); } -/// 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. -pub const MAX_SUBJECT_BYTES: usize = 998; - -/// Upper bound on a single message body. -/// -/// Not a protocol limit -- an application one. Without it a paste of a -/// 50 MB file is attempted verbatim, which stalls the worker and is -/// rejected by every provider anyway. -pub const MAX_BODY_BYTES: usize = 5 * 1024 * 1024; - -/// A non-fatal warning about a config, or None. -/// -/// A4 follow-up / T-E5. Whether `from` may differ from the authenticated -/// identity is the relay's policy, enforced server-side -- we cannot know. -/// But a mismatch is the most common cause of a silent provider rejection, -/// so surfacing it turns "the send failed for no reason" into something -/// actionable. A warning, not an error: some providers legitimately allow -/// send-as aliases. -pub fn config_warning(config: &SmtpConfig) -> Option { - let user = config.username.trim().to_ascii_lowercase(); - let from = config.from.trim().to_ascii_lowercase(); - if user.is_empty() || from.is_empty() || user == from { - return None; - } - // A bare username (no @) is normal for some providers; only warn when - // both look like addresses and they disagree. - if !user.contains('@') { - return None; - } - Some(format!( - "Sending as {from} while signed in as {user}. Some providers reject \ - this unless {from} is a verified alias." - )) -} - -/// Validate the parts of a send that are checkable without a network. -/// -/// A4. Returns the message to show the user, or `Ok(())`. -pub fn validate_send( - config: &SmtpConfig, - to: &str, - subject: &str, - body: &str, -) -> Result<(), String> { - if config.is_incomplete() { - return Err(INCOMPLETE_CONFIG_MESSAGE.to_string()); - } - if to.trim().is_empty() { - return Err("Add at least one recipient.".to_string()); - } - if subject.len() > MAX_SUBJECT_BYTES { - return Err(format!( - "Subject is too long ({} bytes). The limit is {}.", - subject.len(), - MAX_SUBJECT_BYTES - )); - } - if body.len() > MAX_BODY_BYTES { - return Err(format!( - "Message is too large ({} MB). The limit is {} MB.", - body.len() / (1024 * 1024), - MAX_BODY_BYTES / (1024 * 1024) - )); - } - Ok(()) -} - /// Spawn an async email send task. Posts `EmailWorkerAction::SendResult` on completion. -/// -/// A4: validates locally first. Everything checked here is cheap and -/// certain; nothing here needs a server to know it is wrong. pub fn spawn_send_email(config: SmtpConfig, to: String, subject: String, body: String) { - if let Err(msg) = validate_send(&config, &to, &subject, &body) { - Cx::post_action(EmailWorkerAction::SendResult(Err(msg))); - return; - } crate::platform::spawn(async move { let result = send_email_impl(&config, &to, &subject, &body).await; Cx::post_action(EmailWorkerAction::SendResult(result)); @@ -194,124 +49,65 @@ pub fn spawn_send_email(config: SmtpConfig, to: String, subject: String, body: S #[cfg(not(target_arch = "wasm32"))] use lettre::{ + AsyncSmtpTransport, AsyncTransport, Tokio1Executor, message::{Mailbox, Message}, transport::smtp::authentication::Credentials, - AsyncSmtpTransport, AsyncTransport, Tokio1Executor, }; #[cfg(not(target_arch = "wasm32"))] async fn smtp_test_impl(config: &SmtpConfig) -> Result<(), String> { - let creds = Credentials::new(config.username.clone(), config.password.expose().to_owned()); + let creds = Credentials::new(config.username.clone(), config.password.clone()); let mailer = build_transport(config, creds)?; - mailer - .test_connection() - .await + mailer.test_connection().await .map(|_| ()) .map_err(|e| format!("SMTP test failed: {e}")) } #[cfg(not(target_arch = "wasm32"))] -async fn send_email_impl( - config: &SmtpConfig, - to: &str, - subject: &str, - body: &str, -) -> Result<(), String> { - let from_mbox: Mailbox = config - .from - .parse() +async fn send_email_impl(config: &SmtpConfig, to: &str, subject: &str, body: &str) -> Result<(), String> { + let from_mbox: Mailbox = config.from.parse() .map_err(|e| format!("Invalid from: {e}"))?; - let to_mbox: Mailbox = to.parse().map_err(|e| format!("Invalid to: {e}"))?; + let to_mbox: Mailbox = to.parse() + .map_err(|e| format!("Invalid to: {e}"))?; let email = Message::builder() .from(from_mbox) .to(to_mbox) .subject(subject) .body(body.to_owned()) .map_err(|e| format!("Build error: {e}"))?; - let creds = Credentials::new(config.username.clone(), config.password.expose().to_owned()); + let creds = Credentials::new(config.username.clone(), config.password.clone()); let mailer = build_transport(config, creds)?; - mailer - .send(email) - .await + mailer.send(email).await .map_err(|e| format!("Send failed: {e}"))?; Ok(()) } -/// Which TLS mode a port implies. -/// -/// A3: the transport policy used to be implicit -- inherited from -/// `lettre`'s defaults via a port `match` with no statement of intent. -/// Those defaults are in fact safe (`TlsParameters::new` sets -/// `accept_invalid_certs: false`, `accept_invalid_hostnames: false` and a -/// TLS 1.2 floor), but nothing in this repository asserted it, so a -/// refactor to `builder_dangerous(..)` without `.tls(..)` would have -/// removed encryption with no test failing. -/// -/// Naming the policy makes it assertable. See `tls_mode_for_port`. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum TlsMode { - /// Implicit TLS from the first byte (SMTPS). Port 465. - Implicit, - /// Plain connect, then a mandatory STARTTLS upgrade. Ports 587, 25 and - /// anything else. `lettre`'s `starttls_relay` uses `Tls::Required`, so - /// it aborts before sending AUTH if the upgrade fails -- which is what - /// protects against a downgrade attack. - StartTls, -} - -/// Map a port onto its TLS mode. -/// -/// Pure, so the policy can be unit tested on the host without a server. -/// There is deliberately no third arm: every port gets TLS. A cleartext -/// SMTP mode is not offered, because this transport always carries -/// credentials. -pub fn tls_mode_for_port(port: u16) -> TlsMode { - match port { - IMPLICIT_TLS_PORT => TlsMode::Implicit, - _ => TlsMode::StartTls, - } -} - -/// Implicit-TLS submission port. -pub const IMPLICIT_TLS_PORT: u16 = 465; - #[cfg(not(target_arch = "wasm32"))] fn build_transport( config: &SmtpConfig, creds: Credentials, ) -> Result, String> { - // A2: use lettre's own `relay()` rather than reassembling it. - // - // The old code called `builder_dangerous(server).tls(Tls::Wrapper(..))` - // by hand for port 465. I checked lettre 0.11.23's source: `relay()` is - // implemented as exactly those calls, so the two were equivalent and - // certificate validation WAS enabled -- I had initially written this up - // as a critical vulnerability and was wrong. - // - // It is still worth replacing, for two reasons that are not - // hypothetical: a reviewer reading `builder_dangerous` reasonably - // assumes the worst (I did), and hand-rolling means this code inherits - // nothing if upstream hardens `relay()` in a later version. - let builder = match tls_mode_for_port(config.port) { - TlsMode::Implicit => AsyncSmtpTransport::::relay(&config.server) - .map_err(|e| format!("TLS transport error: {e}"))?, - TlsMode::StartTls => AsyncSmtpTransport::::starttls_relay(&config.server) - .map_err(|e| format!("Transport error: {e}"))?, - }; - Ok(builder - .port(config.port) - .credentials(creds) - // A6: bound the operation. lettre defaults to 60s per COMMAND, so a - // multi-command send against a black-holed host can hang far longer - // than a user will wait, with the UI stuck on "Sending...". - .timeout(Some(std::time::Duration::from_secs(SMTP_TIMEOUT_SECS))) - .build()) + match config.port { + 465 => { + let tls = lettre::transport::smtp::client::TlsParameters::new(config.server.clone()) + .map_err(|e| format!("TLS error: {e}"))?; + Ok(AsyncSmtpTransport::::builder_dangerous(&config.server) + .port(config.port) + .credentials(creds) + .tls(lettre::transport::smtp::client::Tls::Wrapper(tls)) + .build()) + } + _ => { + let builder = AsyncSmtpTransport::::starttls_relay(&config.server) + .map_err(|e| format!("Transport error: {e}"))?; + Ok(builder + .port(config.port) + .credentials(creds) + .build()) + } + } } -/// Per-command SMTP timeout. Shorter than lettre's 60s default: a mobile -/// user on a bad connection needs an error, not a two-minute stall. -pub const SMTP_TIMEOUT_SECS: u64 = 20; - // --- Wasm implementation: HTTP API proxy via fetch --- #[cfg(target_arch = "wasm32")] @@ -322,50 +118,27 @@ static EMAIL_API_URL: OnceLock = OnceLock::new(); #[cfg(target_arch = "wasm32")] fn get_email_api_url() -> &'static str { - EMAIL_API_URL - .get() - .map(|s| s.as_str()) - .unwrap_or("/api/email") -} - -/// Build the JSON body for the proxy API. -/// -/// Field by field, deliberately. `SmtpConfig` no longer derives -/// `Serialize`, so this is the ONLY place the password can cross the -/// network, and it is one grep away for anyone auditing what leaves the -/// device. -#[cfg(target_arch = "wasm32")] -fn api_payload(action: &str, config: &SmtpConfig) -> serde_json::Value { - serde_json::json!({ - "action": action, - "config": { - "server": config.server, - "port": config.port, - "username": config.username, - "from": config.from, - // The one line that transmits the secret. - "password": config.password.expose(), - }, - }) + EMAIL_API_URL.get().map(|s| s.as_str()).unwrap_or("/api/email") } #[cfg(target_arch = "wasm32")] async fn smtp_test_impl(config: &SmtpConfig) -> Result<(), String> { - let body = api_payload("test", config); + let body = serde_json::json!({ + "action": "test", + "config": config, + }); call_email_api(&body.to_string()).await } #[cfg(target_arch = "wasm32")] -async fn send_email_impl( - config: &SmtpConfig, - to: &str, - subject: &str, - body: &str, -) -> Result<(), String> { - let mut payload = api_payload("send", config); - payload["to"] = serde_json::Value::from(to); - payload["subject"] = serde_json::Value::from(subject); - payload["body"] = serde_json::Value::from(body); +async fn send_email_impl(config: &SmtpConfig, to: &str, subject: &str, body: &str) -> Result<(), String> { + let payload = serde_json::json!({ + "action": "send", + "config": config, + "to": to, + "subject": subject, + "body": body, + }); call_email_api(&payload.to_string()).await } @@ -405,16 +178,12 @@ async fn call_email_api(json_body: &str) -> Result<(), String> { } else { let status = resp.status(); let text = JsFuture::from( - resp.text() - .map_err(|_| "Failed to read response body".to_string())?, + resp.text().map_err(|_| "Failed to read response body".to_string())?, ) .await .map_err(|e| format!("Failed to read response: {:?}", e))?; - Err(format!( - "Email API error ({}): {}", - status, - text.as_string().unwrap_or_default() - )) + Err(format!("Email API error ({}): {}", status, + text.as_string().unwrap_or_default())) } } @@ -439,236 +208,3 @@ impl ActionDefaultRef for EmailWorkerAction { &NONE } } - -#[cfg(test)] -mod tests { - use super::*; - - fn good() -> SmtpConfig { - SmtpConfig { - server: "smtp.example.com".into(), - port: SmtpConfig::DEFAULT_PORT, - username: "jane@example.com".into(), - password: Secret::new("hunter2"), - from: "jane@example.com".into(), - } - } - - // ---- A1: the secret must not leak --------------------------------- - - /// The config is Debug-printed in error paths and logs. It must not - /// carry the password out with it. - #[test] - fn debug_printing_the_config_does_not_leak_the_password() { - let rendered = format!("{:?}", good()); - assert!( - rendered.contains("smtp.example.com"), - "should show non-secrets" - ); - assert!(!rendered.contains("hunter2"), "leaked: {rendered}"); - } - - // ---- A3: TLS policy is now asserted, not inherited ---------------- - - #[test] - fn port_465_uses_implicit_tls() { - assert_eq!(tls_mode_for_port(465), TlsMode::Implicit); - assert_eq!(tls_mode_for_port(IMPLICIT_TLS_PORT), TlsMode::Implicit); - } - - /// Everything else upgrades via STARTTLS. There is deliberately no - /// cleartext mode: this transport always carries credentials. - #[test] - fn every_other_port_requires_starttls() { - for port in [25u16, 587, 2525, 8465, 1, 65535] { - assert_eq!( - tls_mode_for_port(port), - TlsMode::StartTls, - "port {port} must still require TLS" - ); - } - } - - #[test] - fn the_timeout_is_shorter_than_lettres_default() { - // lettre defaults to 60s per command; a mobile user needs an error - // sooner than that (A6). - assert!(SMTP_TIMEOUT_SECS < 60); - assert!( - SMTP_TIMEOUT_SECS >= 5, - "not so short that a slow relay fails" - ); - } - - // ---- A4: local validation before spending a round trip ------------ - - #[test] - fn a_complete_config_is_not_incomplete() { - assert!(!good().is_incomplete()); - } - - #[test] - fn each_missing_field_makes_the_config_incomplete() { - let cases: Vec<(&str, SmtpConfig)> = vec![ - ( - "server", - SmtpConfig { - server: " ".into(), - ..good() - }, - ), - ( - "username", - SmtpConfig { - username: String::new(), - ..good() - }, - ), - ( - "from", - SmtpConfig { - from: " ".into(), - ..good() - }, - ), - ( - "password", - SmtpConfig { - password: Secret::new(" "), - ..good() - }, - ), - ("port", SmtpConfig { port: 0, ..good() }), - ]; - for (what, cfg) in cases { - assert!(cfg.is_incomplete(), "missing {what} should be incomplete"); - } - } - - #[test] - fn validate_send_rejects_an_empty_recipient() { - let err = validate_send(&good(), " ", "hi", "body").unwrap_err(); - assert!(err.contains("recipient"), "got: {err}"); - } - - #[test] - fn validate_send_rejects_an_over_long_subject() { - let subject = "a".repeat(MAX_SUBJECT_BYTES + 1); - let err = validate_send(&good(), "a@b.com", &subject, "body").unwrap_err(); - assert!(err.contains("Subject"), "got: {err}"); - } - - #[test] - fn validate_send_accepts_a_subject_at_exactly_the_rfc_limit() { - let subject = "a".repeat(MAX_SUBJECT_BYTES); - assert!(validate_send(&good(), "a@b.com", &subject, "body").is_ok()); - } - - #[test] - fn validate_send_rejects_an_absurdly_large_body() { - let body = "a".repeat(MAX_BODY_BYTES + 1); - let err = validate_send(&good(), "a@b.com", "hi", &body).unwrap_err(); - assert!(err.contains("too large"), "got: {err}"); - } - - #[test] - fn validate_send_accepts_an_ordinary_message() { - assert!(validate_send(&good(), "a@b.com", "Hello", "Body text").is_ok()); - } - - /// The config check must come first: without settings there is nothing - /// to send with, and that is the more actionable message. - #[test] - fn an_incomplete_config_is_reported_before_recipient_problems() { - let cfg = SmtpConfig { - server: String::new(), - ..good() - }; - let err = validate_send(&cfg, "", "", "").unwrap_err(); - assert_eq!(err, INCOMPLETE_CONFIG_MESSAGE); - } - - // ---- A4 follow-up: the from/username mismatch warning ------------- - - #[test] - fn no_warning_when_from_matches_the_username() { - assert!(config_warning(&good()).is_none()); - } - - #[test] - fn warns_when_sending_as_a_different_address() { - let cfg = SmtpConfig { - from: "boss@example.com".into(), - ..good() - }; - let w = config_warning(&cfg).expect("should warn"); - assert!(w.contains("boss@example.com")); - assert!(w.contains("jane@example.com")); - } - - /// Case must not manufacture a warning. - #[test] - fn the_comparison_is_case_insensitive() { - let cfg = SmtpConfig { - from: "JANE@Example.COM".into(), - ..good() - }; - assert!(config_warning(&cfg).is_none()); - } - - /// A bare username is normal for some providers and is not a mismatch. - #[test] - fn a_bare_username_never_warns() { - let cfg = SmtpConfig { - username: "jane".into(), - ..good() - }; - assert!(config_warning(&cfg).is_none()); - } - - #[test] - fn an_empty_field_does_not_warn_that_is_validates_job() { - let cfg = SmtpConfig { - from: String::new(), - ..good() - }; - assert!(config_warning(&cfg).is_none()); - } - - // ---- A5: the proxy endpoint must be https or same-origin ---------- - - #[test] - fn relative_same_origin_paths_are_accepted() { - assert!(email_api_url_is_safe("/api/email")); - assert!(email_api_url_is_safe("/v1/mail/send")); - } - - #[test] - fn https_absolute_urls_are_accepted() { - assert!(email_api_url_is_safe("https://mail.example.com/api")); - } - - /// The whole point: credentials must never go over cleartext http. - #[test] - fn http_and_other_schemes_are_rejected() { - for bad in [ - "http://mail.example.com/api", - "ftp://mail.example.com", - "ws://mail.example.com", - "mail.example.com/api", - "https://", - "", - " ", - ] { - assert!(!email_api_url_is_safe(bad), "should reject {bad:?}"); - } - } - - /// `//host/path` inherits the page scheme, so it is http on an http - /// page. That is exactly the downgrade this check exists to stop, and - /// it is easy to mistake for a relative path. - #[test] - fn protocol_relative_urls_are_rejected() { - assert!(!email_api_url_is_safe("//mail.example.com/api")); - } -} diff --git a/crates/nigig-core/src/lib.rs b/crates/nigig-core/src/lib.rs index 2d6a55f..dc4cb6f 100644 --- a/crates/nigig-core/src/lib.rs +++ b/crates/nigig-core/src/lib.rs @@ -19,7 +19,6 @@ pub mod syncing; #[cfg(not(target_arch = "wasm32"))] pub mod tile_service; pub mod email_account; -pub mod secret; pub mod email_store; pub mod email_worker; diff --git a/crates/nigig-core/src/secret.rs b/crates/nigig-core/src/secret.rs deleted file mode 100644 index 6f0ecb1..0000000 --- a/crates/nigig-core/src/secret.rs +++ /dev/null @@ -1,190 +0,0 @@ -//! A string that does not leak itself. -//! -//! Assessment finding S2: `SmtpConfig` carried `pub password: String` and -//! derived both `Serialize` and `Debug`. Three consequences, all live: -//! -//! 1. On wasm the whole struct is `serde_json`-encoded and POSTed to -//! `/api/email`, so every request carried the plaintext password. Any -//! reverse proxy or APM tool logging request bodies -- which is the -//! default for most -- captured it. -//! 2. `Debug` meant any future `log::debug!("{config:?}")` printed it. -//! Nothing did yet. The type offered no protection, and this is -//! exactly how credentials leak: not deliberately, but because a -//! struct that happened to be printable got printed. -//! 3. `Deserialize` implied an intent to persist. Nothing did yet, which -//! was accidentally the safest property of the whole design. -//! -//! `Secret` fixes the type rather than auditing every call site: -//! -//! * `Debug` renders `Secret("***")` -- always, with no way to opt out. -//! * `Serialize` is NOT implemented. A struct holding a `Secret` cannot -//! be silently serialised; the compiler stops it. This is the point: -//! it is a build failure rather than a code review someone has to -//! remember to do. -//! * `expose()` is the only reader, and it is named to be conspicuous -//! in a diff. -//! -//! Not implemented deliberately: -//! -//! * `Display` -- so `format!("{s}")` cannot print it either. -//! * `Serialize`/`Deserialize` -- see above. Persisting a secret needs -//! the platform keystore (plan A1/C1f), not serde. -//! * Zeroing on drop. That would need a crate like `zeroize`, and -//! honesty matters here: without it the plaintext can persist in freed -//! heap memory. It is a real gap and it is recorded in THREAT_MODEL, -//! rather than pretended away by a type that merely looks careful. - -/// A credential held in memory for the current session. -#[derive(Clone, Default, PartialEq, Eq)] -pub struct Secret(String); - -impl Secret { - pub fn new(value: impl Into) -> Self { - Self(value.into()) - } - - /// Read the plaintext. - /// - /// Named to stand out. Every call is a place the secret enters wider - /// scope, so each should be short-lived and obviously necessary -- - /// handing it to an SMTP `Credentials`, for instance. - pub fn expose(&self) -> &str { - &self.0 - } - - /// A password of only whitespace is a user error, not a credential: - /// some providers accept the AUTH and then fail every send. - pub fn is_blank(&self) -> bool { - self.0.trim().is_empty() - } - - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - /// Length, for a strength hint. Does not expose content. - pub fn len(&self) -> usize { - self.0.chars().count() - } - - /// Overwrite with the empty string. - /// - /// Called when a credential is known to be unusable, so a wrong - /// password is not retained for the process lifetime. See the drop - /// caveat in the module docs -- this reduces the window, it does not - /// scrub freed memory. - pub fn clear(&mut self) { - self.0.clear(); - } -} - -/// Always redacted. There is no verbose mode. -impl std::fmt::Debug for Secret { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("Secret(\"***\")") - } -} - -impl From for Secret { - fn from(s: String) -> Self { - Self(s) - } -} - -impl From<&str> for Secret { - fn from(s: &str) -> Self { - Self(s.to_owned()) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - /// The whole point of the type. - #[test] - fn debug_never_prints_the_secret() { - let s = Secret::new("hunter2"); - let rendered = format!("{s:?}"); - assert!(!rendered.contains("hunter2"), "leaked: {rendered}"); - assert_eq!(rendered, "Secret(\"***\")"); - } - - /// `{:#?}` takes a different formatter path; it must also redact. - #[test] - fn alternate_debug_also_redacts() { - let s = Secret::new("hunter2"); - assert!(!format!("{s:#?}").contains("hunter2")); - } - - /// A secret nested in a derived-Debug struct is the realistic leak: - /// nobody prints the password directly, they print the config. - #[test] - fn a_struct_containing_a_secret_does_not_leak_it_via_derived_debug() { - #[derive(Debug)] - struct Config { - user: String, - password: Secret, - } - let c = Config { - user: "jane".into(), - password: Secret::new("hunter2"), - }; - let rendered = format!("{c:?}"); - assert!(rendered.contains("jane"), "should still show non-secrets"); - assert!(!rendered.contains("hunter2"), "leaked: {rendered}"); - } - - #[test] - fn expose_returns_the_plaintext_for_the_one_legitimate_use() { - assert_eq!(Secret::new("hunter2").expose(), "hunter2"); - } - - #[test] - fn clear_overwrites_the_value() { - let mut s = Secret::new("hunter2"); - s.clear(); - assert!(s.is_empty()); - assert_eq!(s.expose(), ""); - } - - /// Whitespace-only is blank but not empty: some providers accept the - /// AUTH and then fail every send, which is worse than a clear error. - #[test] - fn blank_and_empty_are_different_questions() { - let spaces = Secret::new(" "); - assert!(spaces.is_blank()); - assert!(!spaces.is_empty()); - - let real = Secret::new("hunter2"); - assert!(!real.is_blank()); - assert!(!real.is_empty()); - } - - /// A password is not trimmed anywhere, so length must count what the - /// user actually typed -- and count chars, not bytes, or a passphrase - /// with non-Latin characters reports the wrong strength. - #[test] - fn len_counts_characters_not_bytes() { - assert_eq!(Secret::new("hunter2").len(), 7); - assert_eq!(Secret::new(" pad ").len(), 7); - assert_eq!(Secret::new("pässwörd").len(), 8); - assert_eq!(Secret::new("🔑🔑").len(), 2); - } - - #[test] - fn converts_from_the_string_types_a_text_input_produces() { - assert_eq!(Secret::from("a".to_string()).expose(), "a"); - assert_eq!(Secret::from("b").expose(), "b"); - assert_eq!(Secret::default().expose(), ""); - } - - /// Equality is needed for tests and for "did the password change?" - /// checks. It must compare content, not the redacted rendering, or - /// every secret would compare equal to every other. - #[test] - fn equality_compares_content_not_the_redaction() { - assert_eq!(Secret::new("a"), Secret::new("a")); - assert_ne!(Secret::new("a"), Secret::new("b")); - } -}